[PATCH, committed] rs6000: Don't let swaps pass break multiply low-part (PR101129)

[PATCH, committed] rs6000: Don't let swaps pass break multiply low-part (PR101129)

[Bill Schmidt]

Hi,

Segher preapproved this patch in https://gcc.gnu.org/PR101129. It 
differs slightly from what was posted there, needing an additional test 
to ensure the insn is a SET.  The patch also includes the test case 
provided by the OP.  Bootstrap and regtest succeeded on P9 little-endian.

This bug has been around a long time, so the fix should be backported to 
all open releases.  Is this okay after some burn-in time?

Thanks!
Bill

rs6000: Don't let swaps pass break multiply low-part (PR101129)

2021-07-15  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	* config/rs6000/rs6000-p8swap.c (has_part_mult): New.
	(rs6000_analyze_swaps): Insns containing a subreg of a mult are
	not swappable.

gcc/testsuite/
	* gcc.target/powerpc/pr101129.c: New.

diff --git a/gcc/config/rs6000/rs6000-p8swap.c b/gcc/config/rs6000/rs6000-p8swap.c
index 21cbcb2e28a..6b559aa5061 100644
--- a/gcc/config/rs6000/rs6000-p8swap.c
+++ b/gcc/config/rs6000/rs6000-p8swap.c
@@ -1523,6 +1523,22 @@ replace_swap_with_copy (swap_web_entry *insn_entry, unsigned i)
    insn->set_deleted ();
  }
  
+/* INSN is known to contain a SUBREG, which we can normally handle,
+   but if the SUBREG itself contains a MULT then we need to leave it alone
+   to avoid turning a mult_hipart into a mult_lopart, for example.  */
+static bool
+has_part_mult (rtx_insn *insn)
+{
+  rtx body = PATTERN (insn);
+  if (GET_CODE (body) != SET)
+    return false;
+  rtx src = SET_SRC (body);
+  if (GET_CODE (src) != SUBREG)
+    return false;
+  rtx inner = XEXP (src, 0);
+  return (GET_CODE (inner) == MULT);
+}
+
  /* Make NEW_MEM_EXP's attributes and flags resemble those of
     ORIGINAL_MEM_EXP.  */
  static void
@@ -2501,6 +2517,9 @@ rs6000_analyze_swaps (function *fun)
  		    insn_entry[uid].is_swappable = 0;
  		  else if (special != SH_NONE)
  		    insn_entry[uid].special_handling = special;
+		  else if (insn_entry[uid].contains_subreg
+			   && has_part_mult (insn))
+		    insn_entry[uid].is_swappable = 0;
  		  else if (insn_entry[uid].contains_subreg)
  		    insn_entry[uid].special_handling = SH_SUBREG;
  		}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr101129.c b/gcc/testsuite/gcc.target/powerpc/pr101129.c
new file mode 100644
index 00000000000..1abc12480e4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr101129.c
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+/* { dg-require-effective-target p8vector_hw } */
+/* { dg-options "-mdejagnu-cpu=power8 -O " } */
+
+/* PR101129: The swaps pass was turning a mult-lopart into a mult-hipart.
+   Make sure we aren't doing that anymore.  */
+
+typedef unsigned char u8;
+typedef unsigned char __attribute__((__vector_size__ (8))) U;
+typedef unsigned char __attribute__((__vector_size__ (16))) V;
+typedef unsigned int u32;
+typedef unsigned long long u64;
+typedef __int128 u128;
+
+u8 g;
+U u;
+
+void
+foo0 (u32 u32_0, U *ret)
+{
+  u128 u128_2 = u32_0 * (u128)((V){ 5 } > (u32_0 & 4));
+  u64 u64_r = u128_2 >> 64;
+  u8 u8_r = u64_r + g;
+  *ret = u + u8_r;
+}
+
+int
+main (void)
+{
+  U x;
+  foo0 (7, &x);
+  for (unsigned i = 0; i < sizeof (x); i++)
+    if (x[i] != 0) __builtin_abort();
+  return 0;
+}

Re: [PATCH, committed] rs6000: Don't let swaps pass break multiply low-part (PR101129)

[David Edelsohn]

On Thu, Jul 15, 2021 at 11:25 AM Bill Schmidt <wschmidt@linux.ibm.com> wrote:
>
> Hi,
>
> Segher preapproved this patch in https://gcc.gnu.org/PR101129.  It differs slightly from what was posted there, needing an additional test to ensure the insn is a SET.  The patch also includes the test case provided by the OP.  Bootstrap and regtest succeeded on P9 little-endian.
>
> This bug has been around a long time, so the fix should be backported to all open releases.  Is this okay after some burn-in time?
>
> Thanks!
> Bill
>
> rs6000: Don't let swaps pass break multiply low-part (PR101129)
>
> 2021-07-15  Bill Schmidt  <wschmidt@linux.ibm.com>
>
> gcc/
> * config/rs6000/rs6000-p8swap.c (has_part_mult): New.
> (rs6000_analyze_swaps): Insns containing a subreg of a mult are
> not swappable.
>
> gcc/testsuite/
> * gcc.target/powerpc/pr101129.c: New.

Thanks for fixing this so quickly.

Okay everywhere.

Thanks, David