diff mbox series

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

Message ID 20231025020008.2256911-2-guojiufu@linux.ibm.com
State Unresolved
Headers
Series [1/3] rs6000: update num_insns_constant for 2 insns |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jiufu Guo Oct. 25, 2023, 2 a.m. UTC 
  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(-)

Comments

Kewen.Lin Oct. 25, 2023, 2:33 a.m. UTC | #1 
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?  I think only one "li" or "lis"
beats "pli" since the latter is a prefixed insn, it puts more burdens on insn
decoding.

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)));
>
Jiufu Guo Oct. 25, 2023, 7:37 a.m. UTC | #2 
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)));
>>
diff mbox series

Patch

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));
+    }
+  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)));