* [PATCH][V4] rs6000: Remove unnecessary option manipulation.
@ 2021-11-12 16:12 Martin Liska
2021-11-18 12:45 ` Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liska @ 2021-11-12 16:12 UTC (permalink / raw)
To: gcc-patches; +Cc: segher, Martin Liska
Do not set flag_rename_registers, it's already enabled with EnabledBy(funroll-loops)
in the common.opt file. Use EnabledBy for unroll_only_small_loops which
is a canonical approach how can be make option dependencies.
gcc/ChangeLog:
* config/rs6000/rs6000.c (rs6000_override_options_after_change):
Do not set flag_rename_registers and unroll_only_small_loops.
* config/rs6000/rs6000.opt: Use EnabledBy for unroll_only_small_loops.
---
gcc/config/rs6000/rs6000.c | 7 +------
gcc/config/rs6000/rs6000.opt | 2 +-
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index e4843eb0f1c..5550113a94c 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -3466,13 +3466,8 @@ rs6000_override_options_after_change (void)
/* Explicit -funroll-loops turns -munroll-only-small-loops off, and
turns -frename-registers on. */
if ((OPTION_SET_P (flag_unroll_loops) && flag_unroll_loops)
- || (OPTION_SET_P (flag_unroll_all_loops)
- && flag_unroll_all_loops))
+ || (OPTION_SET_P (flag_unroll_all_loops) && flag_unroll_all_loops))
{
- if (!OPTION_SET_P (unroll_only_small_loops))
- unroll_only_small_loops = 0;
- if (!OPTION_SET_P (flag_rename_registers))
- flag_rename_registers = 1;
if (!OPTION_SET_P (flag_cunroll_grow_size))
flag_cunroll_grow_size = 1;
}
diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
index 9d7878f144a..faeb7423ca7 100644
--- a/gcc/config/rs6000/rs6000.opt
+++ b/gcc/config/rs6000/rs6000.opt
@@ -546,7 +546,7 @@ Target Undocumented Var(rs6000_optimize_swaps) Init(1) Save
Analyze and remove doubleword swaps from VSX computations.
munroll-only-small-loops
-Target Undocumented Var(unroll_only_small_loops) Init(0) Save
+Target Undocumented Var(unroll_only_small_loops) Init(0) Save EnabledBy(funroll-loops)
; Use conservative small loop unrolling.
mpower9-misc
--
2.33.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
2021-11-12 16:12 [PATCH][V4] rs6000: Remove unnecessary option manipulation Martin Liska
@ 2021-11-18 12:45 ` Martin Liška
2021-11-18 18:59 ` Segher Boessenkool
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2021-11-18 12:45 UTC (permalink / raw)
To: gcc-patches; +Cc: segher
@Segher: PING
On 11/12/21 17:12, Martin Liska wrote:
> Do not set flag_rename_registers, it's already enabled with EnabledBy(funroll-loops)
> in the common.opt file. Use EnabledBy for unroll_only_small_loops which
> is a canonical approach how can be make option dependencies.
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000.c (rs6000_override_options_after_change):
> Do not set flag_rename_registers and unroll_only_small_loops.
> * config/rs6000/rs6000.opt: Use EnabledBy for unroll_only_small_loops.
> ---
> gcc/config/rs6000/rs6000.c | 7 +------
> gcc/config/rs6000/rs6000.opt | 2 +-
> 2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index e4843eb0f1c..5550113a94c 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -3466,13 +3466,8 @@ rs6000_override_options_after_change (void)
> /* Explicit -funroll-loops turns -munroll-only-small-loops off, and
> turns -frename-registers on. */
> if ((OPTION_SET_P (flag_unroll_loops) && flag_unroll_loops)
> - || (OPTION_SET_P (flag_unroll_all_loops)
> - && flag_unroll_all_loops))
> + || (OPTION_SET_P (flag_unroll_all_loops) && flag_unroll_all_loops))
> {
> - if (!OPTION_SET_P (unroll_only_small_loops))
> - unroll_only_small_loops = 0;
> - if (!OPTION_SET_P (flag_rename_registers))
> - flag_rename_registers = 1;
> if (!OPTION_SET_P (flag_cunroll_grow_size))
> flag_cunroll_grow_size = 1;
> }
> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
> index 9d7878f144a..faeb7423ca7 100644
> --- a/gcc/config/rs6000/rs6000.opt
> +++ b/gcc/config/rs6000/rs6000.opt
> @@ -546,7 +546,7 @@ Target Undocumented Var(rs6000_optimize_swaps) Init(1) Save
> Analyze and remove doubleword swaps from VSX computations.
>
> munroll-only-small-loops
> -Target Undocumented Var(unroll_only_small_loops) Init(0) Save
> +Target Undocumented Var(unroll_only_small_loops) Init(0) Save EnabledBy(funroll-loops)
> ; Use conservative small loop unrolling.
>
> mpower9-misc
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
2021-11-18 12:45 ` Martin Liška
@ 2021-11-18 18:59 ` Segher Boessenkool
2021-11-19 11:32 ` Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Segher Boessenkool @ 2021-11-18 18:59 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches
Hi!
On Thu, Nov 18, 2021 at 01:45:30PM +0100, Martin Liška wrote:
> @Segher: PING
This is the first time I recieved this.
Please resend, without line wrapping (format=flawed).
Segher
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
2021-11-18 18:59 ` Segher Boessenkool
@ 2021-11-19 11:32 ` Martin Liška
2021-11-19 11:43 ` Segher Boessenkool
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2021-11-19 11:32 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
On 11/18/21 19:59, Segher Boessenkool wrote:
> Please resend, without line wrapping (format=flawed).
Done in the original [v4] email, see here:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584267.html
Martin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
2021-11-19 11:32 ` Martin Liška
@ 2021-11-19 11:43 ` Segher Boessenkool
2021-11-19 12:25 ` Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Segher Boessenkool @ 2021-11-19 11:43 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches
On Fri, Nov 19, 2021 at 12:32:09PM +0100, Martin Liška wrote:
> On 11/18/21 19:59, Segher Boessenkool wrote:
> >Please resend, without line wrapping (format=flawed).
>
> Done in the original [v4] email, see here:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584267.html
Which you didn't send to me. And the archives we have now don't let me
grab the raw mail from there anymore. Please send this to me if you
want it reviewed.
Segher
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
2021-11-19 11:43 ` Segher Boessenkool
@ 2021-11-19 12:25 ` Martin Liška
2021-11-19 12:31 ` Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2021-11-19 12:25 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
On 11/19/21 12:43, Segher Boessenkool wrote:
> On Fri, Nov 19, 2021 at 12:32:09PM +0100, Martin Liška wrote:
>> On 11/18/21 19:59, Segher Boessenkool wrote:
>>> Please resend, without line wrapping (format=flawed).
>>
>> Done in the original [v4] email, see here:
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584267.html
>
> Which you didn't send to me. And the archives we have now don't let me
> grab the raw mail from there anymore. Please send this to me if you
> want it reviewed.
Fine. I've just done that.
Martin
>
>
> Segher
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
2021-11-19 12:25 ` Martin Liška
@ 2021-11-19 12:31 ` Martin Liška
2021-11-19 12:45 ` Segher Boessenkool
0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2021-11-19 12:31 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 281 bytes --]
On 11/19/21 13:25, Martin Liška wrote:
> Fine. I've just done that.
All right, so I can't send an email from my local machine and git imap-send
does not work as it goes through Thunderbird.
So my last attempt is attaching the email so that you can add the .eml file.
Martin
[-- Attachment #2: patch.eml --]
[-- Type: message/rfc822, Size: 3226 bytes --]
From: Martin Liska <mliska@suse.cz>
To: gcc-patches@gcc.gnu.org
Cc: segher@kernel.crashing.org, Martin Liska <mliska@suse.cz>
Subject: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
Date: Fri, 12 Nov 2021 17:12:58 +0100
Message-ID: <20211112161258.9964-1-mliska@suse.cz>
Do not set flag_rename_registers, it's already enabled with EnabledBy(funroll-loops)
in the common.opt file. Use EnabledBy for unroll_only_small_loops which
is a canonical approach how can be make option dependencies.
gcc/ChangeLog:
* config/rs6000/rs6000.c (rs6000_override_options_after_change):
Do not set flag_rename_registers and unroll_only_small_loops.
* config/rs6000/rs6000.opt: Use EnabledBy for unroll_only_small_loops.
---
gcc/config/rs6000/rs6000.c | 7 +------
gcc/config/rs6000/rs6000.opt | 2 +-
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index e4843eb0f1c..5550113a94c 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -3466,13 +3466,8 @@ rs6000_override_options_after_change (void)
/* Explicit -funroll-loops turns -munroll-only-small-loops off, and
turns -frename-registers on. */
if ((OPTION_SET_P (flag_unroll_loops) && flag_unroll_loops)
- || (OPTION_SET_P (flag_unroll_all_loops)
- && flag_unroll_all_loops))
+ || (OPTION_SET_P (flag_unroll_all_loops) && flag_unroll_all_loops))
{
- if (!OPTION_SET_P (unroll_only_small_loops))
- unroll_only_small_loops = 0;
- if (!OPTION_SET_P (flag_rename_registers))
- flag_rename_registers = 1;
if (!OPTION_SET_P (flag_cunroll_grow_size))
flag_cunroll_grow_size = 1;
}
diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
index 9d7878f144a..faeb7423ca7 100644
--- a/gcc/config/rs6000/rs6000.opt
+++ b/gcc/config/rs6000/rs6000.opt
@@ -546,7 +546,7 @@ Target Undocumented Var(rs6000_optimize_swaps) Init(1) Save
Analyze and remove doubleword swaps from VSX computations.
munroll-only-small-loops
-Target Undocumented Var(unroll_only_small_loops) Init(0) Save
+Target Undocumented Var(unroll_only_small_loops) Init(0) Save EnabledBy(funroll-loops)
; Use conservative small loop unrolling.
mpower9-misc
--
2.33.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
2021-11-19 12:31 ` Martin Liška
@ 2021-11-19 12:45 ` Segher Boessenkool
2021-11-19 12:57 ` Martin Liška
0 siblings, 1 reply; 9+ messages in thread
From: Segher Boessenkool @ 2021-11-19 12:45 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches
On Fri, Nov 19, 2021 at 01:31:21PM +0100, Martin Liška wrote:
> All right, so I can't send an email from my local machine and git imap-send
> does not work as it goes through Thunderbird.
Hrm, painful (for you). You should figure out how you can do the basics
of the patch-based workflow that we all have to do.
> So my last attempt is attaching the email so that you can add the .eml file.
I cannot do that, but it is text, so it is inlined fine. Thanks.
This is almost identical to what git format-patch would give you, btw.
> Subject: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
> Do not set flag_rename_registers, it's already enabled with EnabledBy(funroll-loops)
> in the common.opt file. Use EnabledBy for unroll_only_small_loops which
> is a canonical approach how can be make option dependencies.
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000.c (rs6000_override_options_after_change):
> Do not set flag_rename_registers and unroll_only_small_loops.
Please don't end changelog lines in ":", it looks like something is
missing. Changelog lines wrap at 80 cols.
> * config/rs6000/rs6000.opt: Use EnabledBy for unroll_only_small_loops.
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -3466,13 +3466,8 @@ rs6000_override_options_after_change (void)
> /* Explicit -funroll-loops turns -munroll-only-small-loops off, and
> turns -frename-registers on. */
> if ((OPTION_SET_P (flag_unroll_loops) && flag_unroll_loops)
> - || (OPTION_SET_P (flag_unroll_all_loops)
> - && flag_unroll_all_loops))
> + || (OPTION_SET_P (flag_unroll_all_loops) && flag_unroll_all_loops))
You should mention this change in the changelog as well.
> - if (!OPTION_SET_P (unroll_only_small_loops))
> - unroll_only_small_loops = 0;
> - if (!OPTION_SET_P (flag_rename_registers))
> - flag_rename_registers = 1;
> if (!OPTION_SET_P (flag_cunroll_grow_size))
> flag_cunroll_grow_size = 1;
> }
> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
> index 9d7878f144a..faeb7423ca7 100644
> --- a/gcc/config/rs6000/rs6000.opt
> +++ b/gcc/config/rs6000/rs6000.opt
> @@ -546,7 +546,7 @@ Target Undocumented Var(rs6000_optimize_swaps) Init(1) Save
> Analyze and remove doubleword swaps from VSX computations.
>
> munroll-only-small-loops
> -Target Undocumented Var(unroll_only_small_loops) Init(0) Save
> +Target Undocumented Var(unroll_only_small_loops) Init(0) Save EnabledBy(funroll-loops)
> ; Use conservative small loop unrolling.
That is the opposite of the original logic.
Segher
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
2021-11-19 12:45 ` Segher Boessenkool
@ 2021-11-19 12:57 ` Martin Liška
0 siblings, 0 replies; 9+ messages in thread
From: Martin Liška @ 2021-11-19 12:57 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches
On 11/19/21 13:45, Segher Boessenkool wrote:
> On Fri, Nov 19, 2021 at 01:31:21PM +0100, Martin Liška wrote:
>> All right, so I can't send an email from my local machine and git imap-send
>> does not work as it goes through Thunderbird.
>
> Hrm, painful (for you). You should figure out how you can do the basics
> of the patch-based workflow that we all have to do.
Sure, but to be honest others don't have problem with patches attached to an email
(with whatever mime type an email client uses).
>
>> So my last attempt is attaching the email so that you can add the .eml file.
>
> I cannot do that, but it is text, so it is inlined fine. Thanks.
>
> This is almost identical to what git format-patch would give you, btw.
Sure, apparently Redirect plugin in TW should work and preserve the email as you need.
>
>> Subject: [PATCH][V4] rs6000: Remove unnecessary option manipulation.
>
>> Do not set flag_rename_registers, it's already enabled with EnabledBy(funroll-loops)
>> in the common.opt file. Use EnabledBy for unroll_only_small_loops which
>> is a canonical approach how can be make option dependencies.
>>
>> gcc/ChangeLog:
>>
>> * config/rs6000/rs6000.c (rs6000_override_options_after_change):
>> Do not set flag_rename_registers and unroll_only_small_loops.
>
> Please don't end changelog lines in ":", it looks like something is
> missing. Changelog lines wrap at 80 cols.
>
>> * config/rs6000/rs6000.opt: Use EnabledBy for unroll_only_small_loops.
>
>> --- a/gcc/config/rs6000/rs6000.c
>> +++ b/gcc/config/rs6000/rs6000.c
>> @@ -3466,13 +3466,8 @@ rs6000_override_options_after_change (void)
>> /* Explicit -funroll-loops turns -munroll-only-small-loops off, and
>> turns -frename-registers on. */
>> if ((OPTION_SET_P (flag_unroll_loops) && flag_unroll_loops)
>> - || (OPTION_SET_P (flag_unroll_all_loops)
>> - && flag_unroll_all_loops))
>> + || (OPTION_SET_P (flag_unroll_all_loops) && flag_unroll_all_loops))
>
> You should mention this change in the changelog as well.
>
>> - if (!OPTION_SET_P (unroll_only_small_loops))
>> - unroll_only_small_loops = 0;
>> - if (!OPTION_SET_P (flag_rename_registers))
>> - flag_rename_registers = 1;
>> if (!OPTION_SET_P (flag_cunroll_grow_size))
>> flag_cunroll_grow_size = 1;
>> }
>> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
>> index 9d7878f144a..faeb7423ca7 100644
>> --- a/gcc/config/rs6000/rs6000.opt
>> +++ b/gcc/config/rs6000/rs6000.opt
>> @@ -546,7 +546,7 @@ Target Undocumented Var(rs6000_optimize_swaps) Init(1) Save
>> Analyze and remove doubleword swaps from VSX computations.
>>
>> munroll-only-small-loops
>> -Target Undocumented Var(unroll_only_small_loops) Init(0) Save
>> +Target Undocumented Var(unroll_only_small_loops) Init(0) Save EnabledBy(funroll-loops)
>> ; Use conservative small loop unrolling.
>
> That is the opposite of the original logic.
You are correct! Forget about the patch, I don't want it merged any longer.
Martin
>
>
> Segher
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-11-19 12:57 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12 16:12 [PATCH][V4] rs6000: Remove unnecessary option manipulation Martin Liska
2021-11-18 12:45 ` Martin Liška
2021-11-18 18:59 ` Segher Boessenkool
2021-11-19 11:32 ` Martin Liška
2021-11-19 11:43 ` Segher Boessenkool
2021-11-19 12:25 ` Martin Liška
2021-11-19 12:31 ` Martin Liška
2021-11-19 12:45 ` Segher Boessenkool
2021-11-19 12:57 ` Martin Liška
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).