Re: [PATCH 2/3]rs6000: using 'pli' to load 34bit-constant

[Jiufu Guo <guojiufu@linux.ibm.com>]

Hi,

"Kewen.Lin" <linkw@linux.ibm.com> writes:

> on 2023/10/25 10:00, Jiufu Guo wrote:
>> Hi,
>> 
>> For constants with 16bit values, 'li or lis' can be used to generate
>> the value.  For 34bit constant, 'pli' is ok to generate the value.
>> 
>> Bootstrap&regtest pass on ppc64{,le}.
>> Is this ok for trunk?
>> 
>> BR,
>> Jeff (Jiufu Guo)
>> 
>> gcc/ChangeLog:
>> 
>> 	* config/rs6000/rs6000.cc (rs6000_emit_set_long_const): Add code to use
>> 	pli for 34bit constant.
>> 
>> ---
>>  gcc/config/rs6000/rs6000.cc | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
>> index b23ff3d7917..4690384cdbe 100644
>> --- a/gcc/config/rs6000/rs6000.cc
>> +++ b/gcc/config/rs6000/rs6000.cc
>> @@ -10530,7 +10530,11 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
>>    ud3 = (c >> 32) & 0xffff;
>>    ud4 = (c >> 48) & 0xffff;
>> 
>> -  if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
>> +  if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (c))
>> +    {
>> +      emit_move_insn (dest, GEN_INT (c));
>> +    }
>
> Nit: unexpected formatting, no {} needed.
>
> Is there any test case justifying this change?
Great catch! pr93012.c could cover this implicitly, but it only be
changed in the [PATCH 3/3].  I would add a new case for this in this
patch.

> I think only one "li" or "lis"
> beats "pli" since the latter is a prefixed insn, it puts more burdens on insn
> decoding.

Yes! Good news is "emit_move_insn (dest, GEN_INT (c));" is able to
support "li, lis and pli".  The "mov" would match/generate the best
one.

Thanks for your quick review and very helpful comments!

BR,
Jeff (Jiufu Guo)
>
> BR,
> Kewen
>
>> +  else if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
>>        || (ud4 == 0 && ud3 == 0 && ud2 == 0 && ! (ud1 & 0x8000)))
>>      emit_move_insn (dest, GEN_INT (sext_hwi (ud1, 16)));
>>