Re: [PATCH]middle-end: Fix store_bit_field expansions of vector constructors [PR120718]

[Richard Sandiford]

Richard Biener <rguent...@suse.de> writes:
> On Tue, 24 Jun 2025, Richard Sandiford wrote:
>> Richard Biener <rguent...@suse.de> writes:
>> > On Tue, 24 Jun 2025, Richard Sandiford wrote:
>> >> (from h8300).  This is also why simplify_gen_subreg has:
>> >> 
>> >>   if (GET_CODE (op) == SUBREG
>> >>       || GET_CODE (op) == CONCAT
>> >>       || GET_MODE (op) == VOIDmode)
>> >>     return NULL_RTX;
>> >> 
>> >>   if (MODE_COMPOSITE_P (outermode)
>> >>       && (CONST_SCALAR_INT_P (op)
>> >>     || CONST_DOUBLE_AS_FLOAT_P (op)
>> >>     || CONST_FIXED_P (op)
>> >>     || GET_CODE (op) == CONST_VECTOR))
>> >>     return NULL_RTX;
>> >> 
>> >> rather than the !REG_P (op) && !MEM_P (op) that the documentation
>> >> would imply.
>> >
>> > So maybe we can drop the MODE_COMPOSITE_P check here, as said on IRC
>> > we don't seem to ever legitmize constants wrapped in a SUBREG, so
>> > we shouldn't generate a SUBREG of a constant (in the middle-end)?
>> 
>> Hmm, yeah, maybe.  I'd originally rejected that because I assumed
>> the MODE_COMPOSITE_P was there for a reason.  But looking at the
>> history, the check came from c0f772894b6b3cd8ed5c5dd09d0c7917f51cf70f,
>> where the reason given was:
>> 
>>     As for the simplify_gen_subreg change, I think it would be desirable
>>     to just avoid creating SUBREGs of constants on all targets and for all
>>     constants, if simplify_immed_subreg simplified, fine, otherwise punt,
>>     but as we are late in GCC11 development, the patch instead guards this
>>     behavior on MODE_COMPOSITE_P (outermode) - i.e. only conversions to
>>     powerpc{,64,64le} double double long double - and only for the cases 
>> where
>>     simplify_immed_subreg was called.
>
> So then I'd say we want to give
>
>   if (CONSTANT_P (op))
>     return NULL_RTX;
>
> a try?

I suppose we could try it, although then the CONSTANT_P test should
replace the VOIDmode test in:

  if (GET_CODE (op) == SUBREG
      || GET_CODE (op) == CONCAT
      || GET_MODE (op) == VOIDmode)
    return NULL_RTX;

The danger is that CONSTANT_P also includes CONST, SYMBOL_REF, and
LABEL_REF, which AFAIK are not expected to be simplified away.

Richard