diff mbox series

rs6000: Properly default-disable late-combine passes [PR106594, PR115622, PR115633] (was: [PATCH 6/6] Add a late-combine pass [PR106594])

Message ID 87frt1l6ja.fsf@euler.schwinge.ddns.net
State New
Headers show
Series rs6000: Properly default-disable late-combine passes [PR106594, PR115622, PR115633] (was: [PATCH 6/6] Add a late-combine pass [PR106594]) | expand

Commit Message

Thomas Schwinge June 25, 2024, 9:23 a.m. UTC 
Hi!

On 2024-06-25T10:07:47+0100, Richard Sandiford <richard.sandiford@arm.com> wrote:
> Thomas Schwinge <tschwinge@baylibre.com> writes:
>> On 2024-06-20T14:34:18+0100, Richard Sandiford <richard.sandiford@arm.com> wrote:
>>> This patch adds a combine pass that runs late in the pipeline.
>>> [...]
>>
>> Nice!
>>
>>> The patch [...] disables the pass by default on i386, rs6000
>>> and xtensa.
>>
>> Like here:
>>
>>> --- a/gcc/config/i386/i386-options.cc
>>> +++ b/gcc/config/i386/i386-options.cc
>>> @@ -1942,6 +1942,10 @@ ix86_override_options_after_change (void)
>>>  	flag_cunroll_grow_size = flag_peel_loops || optimize >= 3;
>>>      }
>>>  
>>> +  /* Late combine tends to undo some of the effects of STV and RPAD,
>>> +     by combining instructions back to their original form.  */
>>> +  if (!OPTION_SET_P (flag_late_combine_instructions))
>>> +    flag_late_combine_instructions = 0;
>>>  }
>>
>> ..., I think also here:
>>
>>> --- a/gcc/config/rs6000/rs6000.cc
>>> +++ b/gcc/config/rs6000/rs6000.cc
>>> @@ -4768,6 +4768,14 @@ rs6000_option_override_internal (bool global_init_p)
>>>  	targetm.expand_builtin_va_start = NULL;
>>>      }
>>>  
>>> +  /* One of the late-combine passes runs after register allocation
>>> +     and can match define_insn_and_splits that were previously used
>>> +     only before register allocation.  Some of those define_insn_and_splits
>>> +     use gen_reg_rtx unconditionally.  Disable late-combine by default
>>> +     until the define_insn_and_splits are fixed.  */
>>> +  if (!OPTION_SET_P (flag_late_combine_instructions))
>>> +    flag_late_combine_instructions = 0;
>>> +
>>>    rs6000_override_options_after_change ();
>>
>> ..., this needs to be done in 'rs6000_override_options_after_change'
>> instead of 'rs6000_option_override_internal', to address the PRs under
>> discussion.  I'm testing such a patch.
>
> Oops!  Sorry about that, and thanks for tracking it down.

No worries.  ;-) OK to push the attached
"rs6000: Properly default-disable late-combine passes [PR106594, PR115622, PR115633]"?


Grüße
 Thomas

Comments

Richard Sandiford June 25, 2024, 9:28 a.m. UTC | #1 
Thomas Schwinge <tschwinge@baylibre.com> writes:
> Hi!
>
> On 2024-06-25T10:07:47+0100, Richard Sandiford <richard.sandiford@arm.com> wrote:
>> Thomas Schwinge <tschwinge@baylibre.com> writes:
>>> On 2024-06-20T14:34:18+0100, Richard Sandiford <richard.sandiford@arm.com> wrote:
>>>> This patch adds a combine pass that runs late in the pipeline.
>>>> [...]
>>>
>>> Nice!
>>>
>>>> The patch [...] disables the pass by default on i386, rs6000
>>>> and xtensa.
>>>
>>> Like here:
>>>
>>>> --- a/gcc/config/i386/i386-options.cc
>>>> +++ b/gcc/config/i386/i386-options.cc
>>>> @@ -1942,6 +1942,10 @@ ix86_override_options_after_change (void)
>>>>  	flag_cunroll_grow_size = flag_peel_loops || optimize >= 3;
>>>>      }
>>>>  
>>>> +  /* Late combine tends to undo some of the effects of STV and RPAD,
>>>> +     by combining instructions back to their original form.  */
>>>> +  if (!OPTION_SET_P (flag_late_combine_instructions))
>>>> +    flag_late_combine_instructions = 0;
>>>>  }
>>>
>>> ..., I think also here:
>>>
>>>> --- a/gcc/config/rs6000/rs6000.cc
>>>> +++ b/gcc/config/rs6000/rs6000.cc
>>>> @@ -4768,6 +4768,14 @@ rs6000_option_override_internal (bool global_init_p)
>>>>  	targetm.expand_builtin_va_start = NULL;
>>>>      }
>>>>  
>>>> +  /* One of the late-combine passes runs after register allocation
>>>> +     and can match define_insn_and_splits that were previously used
>>>> +     only before register allocation.  Some of those define_insn_and_splits
>>>> +     use gen_reg_rtx unconditionally.  Disable late-combine by default
>>>> +     until the define_insn_and_splits are fixed.  */
>>>> +  if (!OPTION_SET_P (flag_late_combine_instructions))
>>>> +    flag_late_combine_instructions = 0;
>>>> +
>>>>    rs6000_override_options_after_change ();
>>>
>>> ..., this needs to be done in 'rs6000_override_options_after_change'
>>> instead of 'rs6000_option_override_internal', to address the PRs under
>>> discussion.  I'm testing such a patch.
>>
>> Oops!  Sorry about that, and thanks for tracking it down.
>
> No worries.  ;-) OK to push the attached
> "rs6000: Properly default-disable late-combine passes [PR106594, PR115622, PR115633]"?

Yes, thanks.

Richard

> Grüße
>  Thomas
>
>
> From ccd12107fb06017f878384d2186ed5f01a1dab79 Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge <tschwinge@baylibre.com>
> Date: Tue, 25 Jun 2024 10:55:41 +0200
> Subject: [PATCH] rs6000: Properly default-disable late-combine passes
>  [PR106594, PR115622, PR115633]
>
> ..., so that it also works for '__attribute__ ((optimize("[...]")))' etc.
>
> 	PR target/106594
> 	PR target/115622
> 	PR target/115633
> 	gcc/
> 	* config/rs6000/rs6000.cc (rs6000_option_override_internal): Move
> 	default-disable of late-combine passes from here...
> 	(rs6000_override_options_after_change): ... to here.
> ---
>  gcc/config/rs6000/rs6000.cc | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index f39b8909925..713fac75f26 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -3431,6 +3431,14 @@ rs6000_override_options_after_change (void)
>    /* If we are inserting ROP-protect instructions, disable shrink wrap.  */
>    if (rs6000_rop_protect)
>      flag_shrink_wrap = 0;
> +
> +  /* One of the late-combine passes runs after register allocation
> +     and can match define_insn_and_splits that were previously used
> +     only before register allocation.  Some of those define_insn_and_splits
> +     use gen_reg_rtx unconditionally.  Disable late-combine by default
> +     until the define_insn_and_splits are fixed.  */
> +  if (!OPTION_SET_P (flag_late_combine_instructions))
> +    flag_late_combine_instructions = 0;
>  }
>  
>  #ifdef TARGET_USES_LINUX64_OPT
> @@ -4768,14 +4776,6 @@ rs6000_option_override_internal (bool global_init_p)
>  	targetm.expand_builtin_va_start = NULL;
>      }
>  
> -  /* One of the late-combine passes runs after register allocation
> -     and can match define_insn_and_splits that were previously used
> -     only before register allocation.  Some of those define_insn_and_splits
> -     use gen_reg_rtx unconditionally.  Disable late-combine by default
> -     until the define_insn_and_splits are fixed.  */
> -  if (!OPTION_SET_P (flag_late_combine_instructions))
> -    flag_late_combine_instructions = 0;
> -
>    rs6000_override_options_after_change ();
>  
>    /* If not explicitly specified via option, decide whether to generate indexed
diff mbox series

Patch

From ccd12107fb06017f878384d2186ed5f01a1dab79 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tschwinge@baylibre.com>
Date: Tue, 25 Jun 2024 10:55:41 +0200
Subject: [PATCH] rs6000: Properly default-disable late-combine passes
 [PR106594, PR115622, PR115633]

..., so that it also works for '__attribute__ ((optimize("[...]")))' etc.

	PR target/106594
	PR target/115622
	PR target/115633
	gcc/
	* config/rs6000/rs6000.cc (rs6000_option_override_internal): Move
	default-disable of late-combine passes from here...
	(rs6000_override_options_after_change): ... to here.
---
 gcc/config/rs6000/rs6000.cc | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index f39b8909925..713fac75f26 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -3431,6 +3431,14 @@  rs6000_override_options_after_change (void)
   /* If we are inserting ROP-protect instructions, disable shrink wrap.  */
   if (rs6000_rop_protect)
     flag_shrink_wrap = 0;
+
+  /* One of the late-combine passes runs after register allocation
+     and can match define_insn_and_splits that were previously used
+     only before register allocation.  Some of those define_insn_and_splits
+     use gen_reg_rtx unconditionally.  Disable late-combine by default
+     until the define_insn_and_splits are fixed.  */
+  if (!OPTION_SET_P (flag_late_combine_instructions))
+    flag_late_combine_instructions = 0;
 }
 
 #ifdef TARGET_USES_LINUX64_OPT
@@ -4768,14 +4776,6 @@  rs6000_option_override_internal (bool global_init_p)
 	targetm.expand_builtin_va_start = NULL;
     }
 
-  /* One of the late-combine passes runs after register allocation
-     and can match define_insn_and_splits that were previously used
-     only before register allocation.  Some of those define_insn_and_splits
-     use gen_reg_rtx unconditionally.  Disable late-combine by default
-     until the define_insn_and_splits are fixed.  */
-  if (!OPTION_SET_P (flag_late_combine_instructions))
-    flag_late_combine_instructions = 0;
-
   rs6000_override_options_after_change ();
 
   /* If not explicitly specified via option, decide whether to generate indexed
-- 
2.34.1