Thomas Schwinge <tschwi...@baylibre.com> writes:
> Hi!
>
> On 2024-06-25T10:07:47+0100, Richard Sandiford <richard.sandif...@arm.com>
> wrote:
>> Thomas Schwinge <tschwi...@baylibre.com> writes:
>>> On 2024-06-20T14:34:18+0100, Richard Sandiford <richard.sandif...@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 <tschwi...@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
