[V4,2/3] Using pli for constant splitting

Message ID 20231211032604.3299841-2-guojiufu@linux.ibm.com
State New
Headers
Series [V4,1/3] rs6000: accurate num_insns_constant_gpr |

Commit Message

Jiufu Guo Dec. 11, 2023, 3:26 a.m. UTC
  Hi,

For constant building e.g. r120=0x66666666, which does not fit 'li or lis',
'pli' is used to build this constant via 'emit_move_insn'.

While for a complicated constant, e.g. 0x6666666666666666ULL, when using
'rs6000_emit_set_long_const' to split the constant recursively, it fails to
use 'pli' to build the half part constant: 0x66666666.

'rs6000_emit_set_long_const' could be updated to use 'pli' to build half
part of the constant when necessary.  For example: 0x6666666666666666ULL,
"pli 3,1717986918; rldimi 3,3,32,0" can be used.

Compare with previous:
https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639492.html
This verion is refreshed and updated testcase name.

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/testsuite/ChangeLog:

	* gcc.target/powerpc/const-build-1.c: New test.

---
 gcc/config/rs6000/rs6000.cc                      | 9 ++++++++-
 gcc/testsuite/gcc.target/powerpc/const-build-1.c | 9 +++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/const-build-1.c
  

Comments

Kewen.Lin Dec. 12, 2023, 2:33 a.m. UTC | #1
Hi,

on 2023/12/11 11:26, Jiufu Guo wrote:
> Hi,
> 
> For constant building e.g. r120=0x66666666, which does not fit 'li or lis',
> 'pli' is used to build this constant via 'emit_move_insn'.
> 
> While for a complicated constant, e.g. 0x6666666666666666ULL, when using
> 'rs6000_emit_set_long_const' to split the constant recursively, it fails to
> use 'pli' to build the half part constant: 0x66666666.
> 
> 'rs6000_emit_set_long_const' could be updated to use 'pli' to build half
> part of the constant when necessary.  For example: 0x6666666666666666ULL,
> "pli 3,1717986918; rldimi 3,3,32,0" can be used.
> 
> Compare with previous:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639492.html
> This verion is refreshed and updated testcase name.
> 
> Bootstrap&regtest pass on ppc64{,le}.
> Is this ok for trunk?

OK for trunk, thanks!

BR,
Kewen

> 
> BR,
> Jeff (Jiufu Guo)
> 
> gcc/ChangeLog:
> 
> 	* config/rs6000/rs6000.cc (rs6000_emit_set_long_const): Add code to use
> 	pli for 34bit constant.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/powerpc/const-build-1.c: New test.
> 
> ---
>  gcc/config/rs6000/rs6000.cc                      | 9 ++++++++-
>  gcc/testsuite/gcc.target/powerpc/const-build-1.c | 9 +++++++++
>  2 files changed, 17 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/const-build-1.c
> 
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 017000a4e02..531c40488b4 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -10511,7 +10511,14 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c, int *num_insns)
>        emit_insn (dest_or_insn);
>    };
> 
> -  if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
> +  if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (c))
> +    {
> +      /* li/lis/pli */
> +      count_or_emit_insn (dest, GEN_INT (c));
> +      return;
> +    }
> +
> + if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
>        || (ud4 == 0 && ud3 == 0 && ud2 == 0 && !(ud1 & 0x8000)))
>      {
>        /* li */
> diff --git a/gcc/testsuite/gcc.target/powerpc/const-build-1.c b/gcc/testsuite/gcc.target/powerpc/const-build-1.c
> new file mode 100644
> index 00000000000..7e35f8c507f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/const-build-1.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
> +/* { dg-require-effective-target power10_ok } */
> +
> +unsigned long long msk66() { return 0x6666666666666666ULL; }
> +
> +/* { dg-final { scan-assembler-times {\mpli\M} 1 } } */
> +/* { dg-final { scan-assembler-not {\mli\M} } } */
> +/* { dg-final { scan-assembler-not {\mlis\M} } } */
  
Jiufu Guo Dec. 13, 2023, 1 a.m. UTC | #2
Hi,

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

> Hi,
>
> on 2023/12/11 11:26, Jiufu Guo wrote:
>> Hi,
>> 
>> For constant building e.g. r120=0x66666666, which does not fit 'li or lis',
>> 'pli' is used to build this constant via 'emit_move_insn'.
>> 
>> While for a complicated constant, e.g. 0x6666666666666666ULL, when using
>> 'rs6000_emit_set_long_const' to split the constant recursively, it fails to
>> use 'pli' to build the half part constant: 0x66666666.
>> 
>> 'rs6000_emit_set_long_const' could be updated to use 'pli' to build half
>> part of the constant when necessary.  For example: 0x6666666666666666ULL,
>> "pli 3,1717986918; rldimi 3,3,32,0" can be used.
>> 
>> Compare with previous:
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639492.html
>> This verion is refreshed and updated testcase name.
>> 
>> Bootstrap&regtest pass on ppc64{,le}.
>> Is this ok for trunk?
>
> OK for trunk, thanks!
Committed via r14-6475.

Thanks for your kind review and great comments!

BR,
Jeff (Jiufu Guo)

>
> BR,
> Kewen
>
>> 
>> BR,
>> Jeff (Jiufu Guo)
>> 
>> gcc/ChangeLog:
>> 
>> 	* config/rs6000/rs6000.cc (rs6000_emit_set_long_const): Add code to use
>> 	pli for 34bit constant.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	* gcc.target/powerpc/const-build-1.c: New test.
>> 
>> ---
>>  gcc/config/rs6000/rs6000.cc                      | 9 ++++++++-
>>  gcc/testsuite/gcc.target/powerpc/const-build-1.c | 9 +++++++++
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/gcc.target/powerpc/const-build-1.c
>> 
>> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
>> index 017000a4e02..531c40488b4 100644
>> --- a/gcc/config/rs6000/rs6000.cc
>> +++ b/gcc/config/rs6000/rs6000.cc
>> @@ -10511,7 +10511,14 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c, int *num_insns)
>>        emit_insn (dest_or_insn);
>>    };
>> 
>> -  if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
>> +  if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (c))
>> +    {
>> +      /* li/lis/pli */
>> +      count_or_emit_insn (dest, GEN_INT (c));
>> +      return;
>> +    }
>> +
>> + if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
>>        || (ud4 == 0 && ud3 == 0 && ud2 == 0 && !(ud1 & 0x8000)))
>>      {
>>        /* li */
>> diff --git a/gcc/testsuite/gcc.target/powerpc/const-build-1.c b/gcc/testsuite/gcc.target/powerpc/const-build-1.c
>> new file mode 100644
>> index 00000000000..7e35f8c507f
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/const-build-1.c
>> @@ -0,0 +1,9 @@
>> +/* { dg-do compile { target lp64 } } */
>> +/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
>> +/* { dg-require-effective-target power10_ok } */
>> +
>> +unsigned long long msk66() { return 0x6666666666666666ULL; }
>> +
>> +/* { dg-final { scan-assembler-times {\mpli\M} 1 } } */
>> +/* { dg-final { scan-assembler-not {\mli\M} } } */
>> +/* { dg-final { scan-assembler-not {\mlis\M} } } */
  

Patch

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 017000a4e02..531c40488b4 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -10511,7 +10511,14 @@  rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c, int *num_insns)
       emit_insn (dest_or_insn);
   };
 
-  if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
+  if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (c))
+    {
+      /* li/lis/pli */
+      count_or_emit_insn (dest, GEN_INT (c));
+      return;
+    }
+
+ if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
       || (ud4 == 0 && ud3 == 0 && ud2 == 0 && !(ud1 & 0x8000)))
     {
       /* li */
diff --git a/gcc/testsuite/gcc.target/powerpc/const-build-1.c b/gcc/testsuite/gcc.target/powerpc/const-build-1.c
new file mode 100644
index 00000000000..7e35f8c507f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/const-build-1.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
+/* { dg-require-effective-target power10_ok } */
+
+unsigned long long msk66() { return 0x6666666666666666ULL; }
+
+/* { dg-final { scan-assembler-times {\mpli\M} 1 } } */
+/* { dg-final { scan-assembler-not {\mli\M} } } */
+/* { dg-final { scan-assembler-not {\mlis\M} } } */