[PATCH] rs6000: Don't ICE when spilling an MMA accumulator

[PATCH] rs6000: Don't ICE when spilling an MMA accumulator

[Peter Bergner]

The following patch fixes one of the bugs discovered in PR96466, namely
when we spill an accumulator that has a known zero value.  In that case,
LRA would emit a new (set (reg:PXI ...) 0) insn, but it would not use the
mma_xxsetaccz pattern to do it.  The solution is to move the xxsetaccz
instruction into the movpxi pattern and have the xxsetaccz pattern call
the move pattern.

This patch fixes the ICE and is in the middle of regression testing.
Ok for trunk if the testing comes back clean?

This is also broken in GCC 10, so ok there after sitting on trunk for
a day or two with no fallout?

Peter

gcc/
	PR target/96446
	* gcc/config/rs6000/mma.md (*movpxi): Add xxsetaccz generation.
	Disable split for zero constant source operand.
	(mma_xxsetaccz): Change to define_expand.  Call gen_movpxi.

gcc/testsuite/
	PR target/96446
	* gcc.target/powerpc/pr96446.c: New test.


diff --git a/gcc/config/rs6000/mma.md b/gcc/config/rs6000/mma.md
index 15cacfb7fc1..fcca02bfa9f 100644
--- a/gcc/config/rs6000/mma.md
+++ b/gcc/config/rs6000/mma.md
@@ -328,11 +328,17 @@
   [(set (match_operand:PXI 0 "nonimmediate_operand" "=d,m,d,d")
 	(match_operand:PXI 1 "input_operand" "m,d,d,O"))]
   "TARGET_MMA
-   && ((gpc_reg_operand (operands[0], PXImode)
-	&& !(CONST_INT_P (operands[1]) && INTVAL (operands[1]) == 0))
+   && (gpc_reg_operand (operands[0], PXImode)
        || gpc_reg_operand (operands[1], PXImode))"
-  "#"
-  "&& reload_completed"
+  "@
+   #
+   #
+   #
+   xxsetaccz %A0"
+  "&& reload_completed
+   && !(fpr_reg_operand (operands[0], PXImode)
+	&& CONST_INT_P (operands[1])
+	&& INTVAL (operands[1]) == 0)"
   [(const_int 0)]
 {
   rs6000_split_multireg_move (operands[0], operands[1]);
@@ -409,12 +415,14 @@
   "<acc> %A0"
   [(set_attr "type" "mma")])
 
-(define_insn "mma_xxsetaccz"
-  [(set (match_operand:PXI 0 "fpr_reg_operand" "=d")
+(define_expand "mma_xxsetaccz"
+  [(set (match_operand:PXI 0 "fpr_reg_operand")
 	(const_int 0))]
   "TARGET_MMA"
-  "xxsetaccz %A0"
-  [(set_attr "type" "mma")])
+{
+  emit_insn (gen_movpxi (operands[0], const0_rtx));
+  DONE;
+})
 
 (define_insn "mma_<vv>"
   [(set (match_operand:PXI 0 "fpr_reg_operand" "=&d")
diff --git a/gcc/testsuite/gcc.target/powerpc/pr96446.c b/gcc/testsuite/gcc.target/powerpc/pr96446.c
new file mode 100644
index 00000000000..2083bf4a76a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr96446.c
@@ -0,0 +1,16 @@
+/* PR target/96466 */
+/* { dg-do compile } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-mdejagnu-cpu=power10 -O2" } */
+
+/* Verify we do not ICE on the following.  */
+
+extern void bar0 (void);
+void
+foo0 (__vector_quad *dst)
+{
+  __vector_quad acc;
+  __builtin_mma_xxsetaccz (&acc);
+  bar0 ();
+  *dst = acc;
+}

Re: [PATCH] rs6000: Don't ICE when spilling an MMA accumulator

[Peter Bergner]

On 8/5/20 2:02 PM, Peter Bergner wrote:
> This patch fixes the ICE and is in the middle of regression testing.
> Ok for trunk if the testing comes back clean?

Testing was clean with no regressions.

Peter

Re: [PATCH] rs6000: Don't ICE when spilling an MMA accumulator

[Segher Boessenkool]

Hi!

On Wed, Aug 05, 2020 at 02:02:36PM -0500, Peter Bergner wrote:
> The following patch fixes one of the bugs discovered in PR96466, namely
> when we spill an accumulator that has a known zero value.  In that case,
> LRA would emit a new (set (reg:PXI ...) 0) insn, but it would not use the
> mma_xxsetaccz pattern to do it.  The solution is to move the xxsetaccz
> instruction into the movpxi pattern and have the xxsetaccz pattern call
> the move pattern.

>    "TARGET_MMA
> -   && ((gpc_reg_operand (operands[0], PXImode)
> -	&& !(CONST_INT_P (operands[1]) && INTVAL (operands[1]) == 0))
> +   && (gpc_reg_operand (operands[0], PXImode)
>         || gpc_reg_operand (operands[1], PXImode))"

Much nicer now :-)

> +  "@
> +   #
> +   #
> +   #
> +   xxsetaccz %A0"
> +  "&& reload_completed
> +   && !(fpr_reg_operand (operands[0], PXImode)
> +	&& CONST_INT_P (operands[1])
> +	&& INTVAL (operands[1]) == 0)"

You can just say

   && reload_completed
   && !(fpr_reg_operand (operands[0], PXImode) && operands[1] == const0_rtx)

afaics?

Okay (for trunk, and later 10) with or without such a change.  Thanks!


Segher

Re: [PATCH] rs6000: Don't ICE when spilling an MMA accumulator

[Peter Bergner]

On 8/5/20 6:06 PM, Segher Boessenkool wrote:
> You can just say
> 
>    && reload_completed
>    && !(fpr_reg_operand (operands[0], PXImode) && operands[1] == const0_rtx)
> 
> afaics?

Agreed.  I made that change and retested which was clean.


> Okay (for trunk, and later 10) with or without such a change.  Thanks!

Ok, updated patch pushed to trunk.  I'll push to GCC10 after a day or two.
Thanks!

Peter

Re: [PATCH] rs6000: Don't ICE when spilling an MMA accumulator

[Peter Bergner]

On 8/6/20 10:29 AM, Peter Bergner wrote:
> On 8/5/20 6:06 PM, Segher Boessenkool wrote:
> Ok, updated patch pushed to trunk.  I'll push to GCC10 after a day or two.

And now pushed to GCC 10.

Peter