diff mbox series

[V4,1/4] rs6000: build constant via li;rotldi

Message ID 20230704021832.1580584-1-guojiufu@linux.ibm.com
State New
Headers show
Series [V4,1/4] rs6000: build constant via li;rotldi | expand

Commit Message

Jiufu Guo July 4, 2023, 2:18 a.m. UTC 
Hi,

If a constant is possible to be rotated to/from a positive or negative
value from "li", then "li;rotldi" can be used to build the constant.

Compare with the previous version:
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621961.html
This patch just did minor changes to the style and comments.

Bootstrap and regtest pass on ppc64{,le}.

Since the previous version is approved with conditions, this version
explained the concern too.  If no objection, I would like to apply
this patch to trunk.


BR,
Jeff (Jiufu)

gcc/ChangeLog:

	* config/rs6000/rs6000.cc (can_be_built_by_li_and_rotldi): New function.
	(rs6000_emit_set_long_const): Call can_be_built_by_li_and_rotldi.

gcc/testsuite/ChangeLog:

	* gcc.target/powerpc/const-build.c: New test.
---
 gcc/config/rs6000/rs6000.cc                   | 47 +++++++++++++--
 .../gcc.target/powerpc/const-build.c          | 57 +++++++++++++++++++
 2 files changed, 98 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/const-build.c

Comments

Kewen.Lin July 4, 2023, 3:28 a.m. UTC | #1 
Hi Jeff,

on 2023/7/4 10:18, Jiufu Guo via Gcc-patches wrote:
> Hi,
> 
> If a constant is possible to be rotated to/from a positive or negative
> value from "li", then "li;rotldi" can be used to build the constant.
> 
> Compare with the previous version:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621961.html
> This patch just did minor changes to the style and comments.
> 
> Bootstrap and regtest pass on ppc64{,le}.
> 
> Since the previous version is approved with conditions, this version
> explained the concern too.  If no objection, I would like to apply
> this patch to trunk.
> 
> 
> BR,
> Jeff (Jiufu)
> 
> gcc/ChangeLog:
> 
> 	* config/rs6000/rs6000.cc (can_be_built_by_li_and_rotldi): New function.
> 	(rs6000_emit_set_long_const): Call can_be_built_by_li_and_rotldi.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/powerpc/const-build.c: New test.
> ---
>  gcc/config/rs6000/rs6000.cc                   | 47 +++++++++++++--
>  .../gcc.target/powerpc/const-build.c          | 57 +++++++++++++++++++
>  2 files changed, 98 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/const-build.c
> 
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 42f49e4a56b..acc332acc05 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -10258,6 +10258,31 @@ rs6000_emit_set_const (rtx dest, rtx source)
>    return true;
>  }
>  
> +/* Check if value C can be built by 2 instructions: one is 'li', another is
> +   rotldi.

Nit: different style, li is with "'" but rotldi isn't.

> +
> +   If so, *SHIFT is set to the shift operand of rotldi(rldicl), and *MASK
> +   is set to the mask operand of rotldi(rldicl), and return true.
> +   Return false otherwise.  */
> +
> +static bool
> +can_be_built_by_li_and_rotldi (HOST_WIDE_INT c, int *shift,
> +				   HOST_WIDE_INT *mask)
> +{
> +  /* If C or ~C contains at least 49 successive zeros, then C can be rotated
> +     to/from a positive or negative value that 'li' is able to load.  */
> +  int n;
> +  if (can_be_rotated_to_lowbits (c, 15, &n)
> +      || can_be_rotated_to_lowbits (~c, 15, &n))
> +    {
> +      *mask = HOST_WIDE_INT_M1;
> +      *shift = HOST_BITS_PER_WIDE_INT - n;
> +      return true;
> +    }
> +
> +  return false;
> +}
> +
>  /* Subroutine of rs6000_emit_set_const, handling PowerPC64 DImode.
>     Output insns to set DEST equal to the constant C as a series of
>     lis, ori and shl instructions.  */
> @@ -10266,15 +10291,14 @@ static void
>  rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
>  {
>    rtx temp;
> +  int shift;
> +  HOST_WIDE_INT mask;
>    HOST_WIDE_INT ud1, ud2, ud3, ud4;
>  
>    ud1 = c & 0xffff;
> -  c = c >> 16;
> -  ud2 = c & 0xffff;
> -  c = c >> 16;
> -  ud3 = c & 0xffff;
> -  c = c >> 16;
> -  ud4 = c & 0xffff;
> +  ud2 = (c >> 16) & 0xffff;
> +  ud3 = (c >> 32) & 0xffff;
> +  ud4 = (c >> 48) & 0xffff;
>  
>    if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
>        || (ud4 == 0 && ud3 == 0 && ud2 == 0 && ! (ud1 & 0x8000)))
> @@ -10305,6 +10329,17 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
>        emit_move_insn (dest, gen_rtx_XOR (DImode, temp,
>  					 GEN_INT ((ud2 ^ 0xffff) << 16)));
>      }
> +  else if (can_be_built_by_li_and_rotldi (c, &shift, &mask))
> +    {
> +      temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
> +      unsigned HOST_WIDE_INT imm = (c | ~mask);
> +      imm = (imm >> shift) | (imm << (HOST_BITS_PER_WIDE_INT - shift));
> +
> +      emit_move_insn (temp, GEN_INT (imm));
> +      if (shift != 0)
> +	temp = gen_rtx_ROTATE (DImode, temp, GEN_INT (shift));
> +      emit_move_insn (dest, temp);
> +    }
>    else if (ud3 == 0 && ud4 == 0)
>      {
>        temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
> diff --git a/gcc/testsuite/gcc.target/powerpc/const-build.c b/gcc/testsuite/gcc.target/powerpc/const-build.c
> new file mode 100644
> index 00000000000..69b37e2bb53
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/const-build.c
> @@ -0,0 +1,57 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2 -save-temps" } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +
> +/* Verify that two instructions are sucessfully used to build constants.

s/sucessfully/successfully/

> +   One insn is li or lis, another is rotate: rldicl, rldicr or rldic.  */

Nit: This patch is for insn li + insn rldicl only, you probably want to keep
consistent in the comments.

The others look good to me, thanks!

Segher had one question on "~c" before, I saw you had explained for it, it
makes sense to me, but in case he has more questions I'd defer the final
approval to him.

BR,
Kewen
Jiufu Guo Aug. 18, 2023, 6:05 a.m. UTC | #2 
Hi Segher,

As discussed on "~" vs. "-",  "~" is correct for this patch.

I updated the patch according to Kewen's comments.

If ok,  I would commit to trunk.

BR,
Jeff (Jiufu Guo)


On 2023-07-04 11:28, Kewen.Lin wrote:
> Hi Jeff,
> 
> on 2023/7/4 10:18, Jiufu Guo via Gcc-patches wrote:
>> Hi,
>> 
>> If a constant is possible to be rotated to/from a positive or negative
>> value from "li", then "li;rotldi" can be used to build the constant.
>> 
>> Compare with the previous version:
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621961.html
>> This patch just did minor changes to the style and comments.
>> 
>> Bootstrap and regtest pass on ppc64{,le}.
>> 
>> Since the previous version is approved with conditions, this version
>> explained the concern too.  If no objection, I would like to apply
>> this patch to trunk.
>> 
>> 
>> BR,
>> Jeff (Jiufu)
>> 
>> gcc/ChangeLog:
>> 
>> 	* config/rs6000/rs6000.cc (can_be_built_by_li_and_rotldi): New 
>> function.
>> 	(rs6000_emit_set_long_const): Call can_be_built_by_li_and_rotldi.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	* gcc.target/powerpc/const-build.c: New test.
>> ---
>>  gcc/config/rs6000/rs6000.cc                   | 47 +++++++++++++--
>>  .../gcc.target/powerpc/const-build.c          | 57 
>> +++++++++++++++++++
>>  2 files changed, 98 insertions(+), 6 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.target/powerpc/const-build.c
>> 
>> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
>> index 42f49e4a56b..acc332acc05 100644
>> --- a/gcc/config/rs6000/rs6000.cc
>> +++ b/gcc/config/rs6000/rs6000.cc
>> @@ -10258,6 +10258,31 @@ rs6000_emit_set_const (rtx dest, rtx source)
>>    return true;
>>  }
>> 
>> +/* Check if value C can be built by 2 instructions: one is 'li', 
>> another is
>> +   rotldi.
> 
> Nit: different style, li is with "'" but rotldi isn't.
> 
>> +
>> +   If so, *SHIFT is set to the shift operand of rotldi(rldicl), and 
>> *MASK
>> +   is set to the mask operand of rotldi(rldicl), and return true.
>> +   Return false otherwise.  */
>> +
>> +static bool
>> +can_be_built_by_li_and_rotldi (HOST_WIDE_INT c, int *shift,
>> +				   HOST_WIDE_INT *mask)
>> +{
>> +  /* If C or ~C contains at least 49 successive zeros, then C can be 
>> rotated
>> +     to/from a positive or negative value that 'li' is able to load.  
>> */
>> +  int n;
>> +  if (can_be_rotated_to_lowbits (c, 15, &n)
>> +      || can_be_rotated_to_lowbits (~c, 15, &n))
>> +    {
>> +      *mask = HOST_WIDE_INT_M1;
>> +      *shift = HOST_BITS_PER_WIDE_INT - n;
>> +      return true;
>> +    }
>> +
>> +  return false;
>> +}
>> +
>>  /* Subroutine of rs6000_emit_set_const, handling PowerPC64 DImode.
>>     Output insns to set DEST equal to the constant C as a series of
>>     lis, ori and shl instructions.  */
>> @@ -10266,15 +10291,14 @@ static void
>>  rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
>>  {
>>    rtx temp;
>> +  int shift;
>> +  HOST_WIDE_INT mask;
>>    HOST_WIDE_INT ud1, ud2, ud3, ud4;
>> 
>>    ud1 = c & 0xffff;
>> -  c = c >> 16;
>> -  ud2 = c & 0xffff;
>> -  c = c >> 16;
>> -  ud3 = c & 0xffff;
>> -  c = c >> 16;
>> -  ud4 = c & 0xffff;
>> +  ud2 = (c >> 16) & 0xffff;
>> +  ud3 = (c >> 32) & 0xffff;
>> +  ud4 = (c >> 48) & 0xffff;
>> 
>>    if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 
>> 0x8000))
>>        || (ud4 == 0 && ud3 == 0 && ud2 == 0 && ! (ud1 & 0x8000)))
>> @@ -10305,6 +10329,17 @@ rs6000_emit_set_long_const (rtx dest, 
>> HOST_WIDE_INT c)
>>        emit_move_insn (dest, gen_rtx_XOR (DImode, temp,
>>  					 GEN_INT ((ud2 ^ 0xffff) << 16)));
>>      }
>> +  else if (can_be_built_by_li_and_rotldi (c, &shift, &mask))
>> +    {
>> +      temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
>> +      unsigned HOST_WIDE_INT imm = (c | ~mask);
>> +      imm = (imm >> shift) | (imm << (HOST_BITS_PER_WIDE_INT - 
>> shift));
>> +
>> +      emit_move_insn (temp, GEN_INT (imm));
>> +      if (shift != 0)
>> +	temp = gen_rtx_ROTATE (DImode, temp, GEN_INT (shift));
>> +      emit_move_insn (dest, temp);
>> +    }
>>    else if (ud3 == 0 && ud4 == 0)
>>      {
>>        temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
>> diff --git a/gcc/testsuite/gcc.target/powerpc/const-build.c 
>> b/gcc/testsuite/gcc.target/powerpc/const-build.c
>> new file mode 100644
>> index 00000000000..69b37e2bb53
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/const-build.c
>> @@ -0,0 +1,57 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 -save-temps" } */
>> +/* { dg-require-effective-target has_arch_ppc64 } */
>> +
>> +/* Verify that two instructions are sucessfully used to build 
>> constants.
> 
> s/sucessfully/successfully/
> 
>> +   One insn is li or lis, another is rotate: rldicl, rldicr or rldic. 
>>  */
> 
> Nit: This patch is for insn li + insn rldicl only, you probably want to 
> keep
> consistent in the comments.
> 
> The others look good to me, thanks!
> 
> Segher had one question on "~c" before, I saw you had explained for it, 
> it
> makes sense to me, but in case he has more questions I'd defer the 
> final
> approval to him.
> 
> BR,
> Kewen
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 42f49e4a56b..acc332acc05 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -10258,6 +10258,31 @@  rs6000_emit_set_const (rtx dest, rtx source)
   return true;
 }
 
+/* Check if value C can be built by 2 instructions: one is 'li', another is
+   rotldi.
+
+   If so, *SHIFT is set to the shift operand of rotldi(rldicl), and *MASK
+   is set to the mask operand of rotldi(rldicl), and return true.
+   Return false otherwise.  */
+
+static bool
+can_be_built_by_li_and_rotldi (HOST_WIDE_INT c, int *shift,
+				   HOST_WIDE_INT *mask)
+{
+  /* If C or ~C contains at least 49 successive zeros, then C can be rotated
+     to/from a positive or negative value that 'li' is able to load.  */
+  int n;
+  if (can_be_rotated_to_lowbits (c, 15, &n)
+      || can_be_rotated_to_lowbits (~c, 15, &n))
+    {
+      *mask = HOST_WIDE_INT_M1;
+      *shift = HOST_BITS_PER_WIDE_INT - n;
+      return true;
+    }
+
+  return false;
+}
+
 /* Subroutine of rs6000_emit_set_const, handling PowerPC64 DImode.
    Output insns to set DEST equal to the constant C as a series of
    lis, ori and shl instructions.  */
@@ -10266,15 +10291,14 @@  static void
 rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
 {
   rtx temp;
+  int shift;
+  HOST_WIDE_INT mask;
   HOST_WIDE_INT ud1, ud2, ud3, ud4;
 
   ud1 = c & 0xffff;
-  c = c >> 16;
-  ud2 = c & 0xffff;
-  c = c >> 16;
-  ud3 = c & 0xffff;
-  c = c >> 16;
-  ud4 = c & 0xffff;
+  ud2 = (c >> 16) & 0xffff;
+  ud3 = (c >> 32) & 0xffff;
+  ud4 = (c >> 48) & 0xffff;
 
   if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000))
       || (ud4 == 0 && ud3 == 0 && ud2 == 0 && ! (ud1 & 0x8000)))
@@ -10305,6 +10329,17 @@  rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c)
       emit_move_insn (dest, gen_rtx_XOR (DImode, temp,
 					 GEN_INT ((ud2 ^ 0xffff) << 16)));
     }
+  else if (can_be_built_by_li_and_rotldi (c, &shift, &mask))
+    {
+      temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
+      unsigned HOST_WIDE_INT imm = (c | ~mask);
+      imm = (imm >> shift) | (imm << (HOST_BITS_PER_WIDE_INT - shift));
+
+      emit_move_insn (temp, GEN_INT (imm));
+      if (shift != 0)
+	temp = gen_rtx_ROTATE (DImode, temp, GEN_INT (shift));
+      emit_move_insn (dest, temp);
+    }
   else if (ud3 == 0 && ud4 == 0)
     {
       temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode);
diff --git a/gcc/testsuite/gcc.target/powerpc/const-build.c b/gcc/testsuite/gcc.target/powerpc/const-build.c
new file mode 100644
index 00000000000..69b37e2bb53
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/const-build.c
@@ -0,0 +1,57 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2 -save-temps" } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+
+/* Verify that two instructions are sucessfully used to build constants.
+   One insn is li or lis, another is rotate: rldicl, rldicr or rldic.  */
+
+#define NOIPA __attribute__ ((noipa))
+
+struct fun
+{
+  long long (*f) (void);
+  long long val;
+};
+
+long long NOIPA
+li_rotldi_1 (void)
+{
+  return 0x7531000000000LL;
+}
+
+long long NOIPA
+li_rotldi_2 (void)
+{
+  return 0x2100000000000064LL;
+}
+
+long long NOIPA
+li_rotldi_3 (void)
+{
+  return 0xffff8531ffffffffLL;
+}
+
+long long NOIPA
+li_rotldi_4 (void)
+{
+  return 0x21ffffffffffff94LL;
+}
+
+struct fun arr[] = {
+  {li_rotldi_1, 0x7531000000000LL},
+  {li_rotldi_2, 0x2100000000000064LL},
+  {li_rotldi_3, 0xffff8531ffffffffLL},
+  {li_rotldi_4, 0x21ffffffffffff94LL},
+};
+
+/* { dg-final { scan-assembler-times {\mrotldi\M} 4 } } */
+
+int
+main ()
+{
+  for (int i = 0; i < sizeof (arr) / sizeof (arr[0]); i++)
+    if ((*arr[i].f) () != arr[i].val)
+      __builtin_abort ();
+
+  return 0;
+}