[PATCH] RISC-V: Fix bug of VSETVL fusion

[PATCH] RISC-V: Fix bug of VSETVL fusion

[Juzhe-Zhong]

This patch fixes bugs in the fusion of this following case:

li a5,-1
vmv.s.x v0,a5 -> demand any non-zero AVL
vsetvli a5, ...

Incorrect fusion after VSETVL PASS:

li a5,-1
vsetvli a5...
vmv.s.x v0, a5 --> a5 is modified as incorrect value.

We disallow this incorrect fusion above.

Full coverage testing of RV64 and RV32 no regression.

	PR target/113087

gcc/ChangeLog:

	* config/riscv/riscv-vsetvl.cc: Disallow fusion when VL modification pollutes non AVL use.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/autovec/pr113087-1.c: New test.

---
 gcc/config/riscv/riscv-vsetvl.cc              | 41 ++++++++++++-
 .../gcc.target/riscv/rvv/autovec/pr113087-1.c | 60 +++++++++++++++++++
 2 files changed, 99 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113087-1.c

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 6af8d8429ab..eabaef80f89 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -1128,6 +1128,27 @@ public:
       return gen_vsetvl_discard_result (Pmode, avl, sew, vlmul, ta, ma);
   }
 
+  /* Return true that the non-AVL operands of THIS will be modified
+     if we fuse the VL modification from OTHER into THIS.  */
+  bool vl_modify_non_avl_op_p (const vsetvl_info &other) const
+  {
+    /* We don't need to worry about any operands from THIS be
+       modified by OTHER vsetvl since we OTHER vsetvl doesn't
+       modify any operand.  */
+    if (!other.has_vl ())
+      return false;
+
+    /* THIS VL operand always preempt OTHER VL operand.  */
+    if (this->has_vl ())
+      return false;
+
+    /* If THIS has non IMM AVL and THIS is AVL compatible with
+       OTHER, the AVL value of THIS is same as VL value of OTHER.  */
+    if (!this->has_imm_avl ())
+      return false;
+    return find_access (this->get_insn ()->uses (), REGNO (other.get_vl ()));
+  }
+
   bool operator== (const vsetvl_info &other) const
   {
     gcc_assert (!uninit_p () && !other.uninit_p ()
@@ -1896,6 +1917,20 @@ public:
     gcc_unreachable ();
   }
 
+  bool vl_not_in_conflict_p (const vsetvl_info &prev, const vsetvl_info &next)
+  {
+    /* We don't fuse this following case:
+
+	li a5, -1
+	vmv.s.x v0, a5         -- PREV
+	vsetvli a5, ...        -- NEXT
+
+       Don't fuse NEXT into PREV.
+    */
+    return !prev.vl_modify_non_avl_op_p (next)
+	   && !next.vl_modify_non_avl_op_p (prev);
+  }
+
   bool avl_compatible_p (const vsetvl_info &prev, const vsetvl_info &next)
   {
     gcc_assert (prev.valid_p () && next.valid_p ());
@@ -1953,7 +1988,8 @@ public:
   {
     bool compatible_p = sew_lmul_compatible_p (prev, next)
 			&& policy_compatible_p (prev, next)
-			&& avl_compatible_p (prev, next);
+			&& avl_compatible_p (prev, next)
+			&& vl_not_in_conflict_p (prev, next);
     return compatible_p;
   }
 
@@ -1961,7 +1997,8 @@ public:
   {
     bool available_p = sew_lmul_available_p (prev, next)
 		       && policy_available_p (prev, next)
-		       && avl_available_p (prev, next);
+		       && avl_available_p (prev, next)
+		       && vl_not_in_conflict_p (prev, next);
     gcc_assert (!available_p || compatible_p (prev, next));
     return available_p;
   }
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113087-1.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113087-1.c
new file mode 100644
index 00000000000..7b743effc79
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113087-1.c
@@ -0,0 +1,60 @@
+/* { dg-do run } */
+/* { dg-options "-O3" } */
+/* { dg-require-effective-target riscv_v } */
+
+#include <assert.h>
+int (e) (int g, int h) { return h > 0x10 || g > 0xFFFFFFFF >> h ? g : g << h; }
+struct i
+{
+  int j;
+  int l : 1;
+};
+struct m
+{
+  char k;
+  int n;
+};
+char o;
+char p;
+short s;
+int q;
+struct m r;
+int v;
+int t;
+short z;
+long ac;
+int ad;
+int ae;
+
+static void
+ai (struct i bf)
+{
+  for (; v; v++)
+    r.k = 0;
+  do
+    ac ^= bf.j;
+  while (bf.j < 0);
+  s = 0;
+  if (bf.l)
+    q |= 0x800;
+}
+
+int
+main ()
+{
+  struct i aw = {0xE00, 1};
+  o = 4;
+  s = p;
+  ai (aw);
+  t = 1;
+  ++p;
+  for (; t <= 7; t++)
+    {
+      ad &= 1;
+      (o &= 1 - e (0x40000012, ++ae)) & (z |= 1);
+    }
+  for (; r.n;)
+    ;
+  assert (o == 4);
+  return 0;
+}
-- 
2.36.3

Re: [PATCH] RISC-V: Fix bug of VSETVL fusion

[Jeff Law]

On 12/19/23 23:50, Juzhe-Zhong wrote:
> This patch fixes bugs in the fusion of this following case:
> 
> li a5,-1
> vmv.s.x v0,a5 -> demand any non-zero AVL
> vsetvli a5, ...
> 
> Incorrect fusion after VSETVL PASS:
> 
> li a5,-1
> vsetvli a5...
> vmv.s.x v0, a5 --> a5 is modified as incorrect value.
> 
> We disallow this incorrect fusion above.
> 
> Full coverage testing of RV64 and RV32 no regression.
> 
> 	PR target/113087
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv-vsetvl.cc: Disallow fusion when VL modification pollutes non AVL use.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/riscv/rvv/autovec/pr113087-1.c: New test.
OK.
jeff