Re: [PATCH 6/6] Add a late-combine pass [PR106594]

[Richard Sandiford <richard.sandiford@arm.com>]

Thomas Schwinge <tschwinge@baylibre.com> writes:
> Hi!
>
> 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.

Richard