diff mbox series

RISC-V: Do not delete fused vsetvl if it has uses [PR119115].

Message ID D89ZGOTHUWMN.15WFCNJ10TECO@gmail.com
State Committed
Delegated to: Jeff Law
Headers
Series RISC-V: Do not delete fused vsetvl if it has uses [PR119115]. |

Checks

Context Check Description
rivoscibot/toolchain-ci-rivos-lint success Lint passed
rivoscibot/toolchain-ci-rivos-apply-patch success Patch applied
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-multilib success Build passed
rivoscibot/toolchain-ci-rivos-test success Testing passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Robin Dapp March 7, 2025, 11:05 a.m. UTC 
  Hi,

in PR119115 we end up with an orphaned
	vsetvli	zero,t1,e16,m1,ta,ma.
t1 originally came from another vsetvl that was fused from
	vsetvli	a4,a3,e8,mf2,ta,ma
	vsetvli	t1,a3,e8,mf2,ta,ma   (1)
to
	vsetvli	zero,a3,e16,m1,ta,ma.

This patch checks if t1, the VL operand of (1), has AVL uses and does
not delete the vsetvl if so.  While doing so, it also wraps the search
for VL uses into two new functions reg_used and reg_single_use_in_avl.

Regtested on rv64gcv_zvl512b and rv64gcv_zvl256b.  Let's see what the CI says.  
For my last patch it showed an execution failure that I cannot reproduce 
locally.

Regards
 Robin

	PR target/119115

gcc/ChangeLog:

	* config/riscv/riscv-vsetvl.cc (reg_used): New function.
	(reg_single_use_in_avl): Ditto.
	(pre_vsetvl::fuse_local_vsetvl_info): Use reg_single_use_in_avl
	when checking if vsetvl can be deleted.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/base/pr119115.c: New test.
---
 gcc/config/riscv/riscv-vsetvl.cc              | 95 ++++++++++++++-----
 .../gcc.target/riscv/rvv/base/pr119115.c      | 59 ++++++++++++
 2 files changed, 131 insertions(+), 23 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr119115.c

Comments

Jeff Law March 11, 2025, 2:01 p.m. UTC | #1 
On 3/7/25 4:05 AM, Robin Dapp wrote:
> Hi,
> 
> in PR119115 we end up with an orphaned
>      vsetvli    zero,t1,e16,m1,ta,ma.
> t1 originally came from another vsetvl that was fused from
>      vsetvli    a4,a3,e8,mf2,ta,ma
>      vsetvli    t1,a3,e8,mf2,ta,ma   (1)
> to
>      vsetvli    zero,a3,e16,m1,ta,ma.
> 
> This patch checks if t1, the VL operand of (1), has AVL uses and does
> not delete the vsetvl if so.  While doing so, it also wraps the search
> for VL uses into two new functions reg_used and reg_single_use_in_avl.
> 
> Regtested on rv64gcv_zvl512b and rv64gcv_zvl256b.  Let's see what the CI 
> says. For my last patch it showed an execution failure that I cannot 
> reproduce locally.
> 
> Regards
> Robin
> 
>      PR target/119115
> 
> gcc/ChangeLog:
> 
>      * config/riscv/riscv-vsetvl.cc (reg_used): New function.
>      (reg_single_use_in_avl): Ditto.
>      (pre_vsetvl::fuse_local_vsetvl_info): Use reg_single_use_in_avl
>      when checking if vsetvl can be deleted.
> 
> gcc/testsuite/ChangeLog:
> 
>      * gcc.target/riscv/rvv/base/pr119115.c: New test.
OK.  Sorry about the delay.

jeff
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index f0165f7b8c8..64342efb786 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -780,6 +780,36 @@  enum class avl_demand_type : unsigned
   ignore_avl = demand_flags::DEMAND_EMPTY_P,
 };
 
+/* Go through all uses of INSN insn looking for a single use of register REG.
+   Return true if we find
+    - Uses in a non-RVV insn
+    - More than one use in an RVV insn
+    - A single use in the VL operand of an RVV insn
+   and false otherwise.
+   A single use in the AVL operand does not count as use as we take care of
+   those separately in the pass.  */
+
+static bool
+reg_used (insn_info *insn, rtx reg)
+{
+  unsigned int regno = REGNO (reg);
+  const hash_set<use_info *> vl_uses = get_all_real_uses (insn, regno);
+  for (use_info *use : vl_uses)
+    {
+      gcc_assert (use->insn ()->is_real ());
+      rtx_insn *rinsn = use->insn ()->rtl ();
+      if (!has_vl_op (rinsn)
+	  || count_regno_occurrences (rinsn, regno) != 1)
+	return true;
+
+      rtx avl = ::get_avl (rinsn);
+      if (!avl || !REG_P (avl) || regno != REGNO (avl))
+	return true;
+    }
+  return false;
+}
+
+
 class vsetvl_info
 {
 private:
@@ -1142,27 +1172,7 @@  public:
 
     /* Determine if dest operand(vl) has been used by non-RVV instructions.  */
     if (dest_vl)
-      {
-	const hash_set<use_info *> vl_uses
-	  = get_all_real_uses (get_insn (), REGNO (dest_vl));
-	for (use_info *use : vl_uses)
-	  {
-	    gcc_assert (use->insn ()->is_real ());
-	    rtx_insn *rinsn = use->insn ()->rtl ();
-	    if (!has_vl_op (rinsn)
-		|| count_regno_occurrences (rinsn, REGNO (dest_vl)) != 1)
-	      {
-		m_vl_used_by_non_rvv_insn = true;
-		break;
-	      }
-	    rtx avl = ::get_avl (rinsn);
-	    if (!avl || !REG_P (avl) || REGNO (dest_vl) != REGNO (avl))
-	      {
-		m_vl_used_by_non_rvv_insn = true;
-		break;
-	      }
-	  }
-      }
+      m_vl_used_by_non_rvv_insn = reg_used (get_insn (), dest_vl);
 
     /* Collect the read vl insn for the fault-only-first rvv loads.  */
     if (fault_first_load_p (insn->rtl ()))
@@ -1369,6 +1379,35 @@  public:
   void set_empty_info () { global_info.set_empty (); }
 };
 
+/* Same as REG_USED () but looks for a single use in an RVV insn's AVL
+   operand.  */
+static bool
+reg_single_use_in_avl (insn_info *insn, rtx reg)
+{
+  if (!reg)
+    return false;
+  unsigned int regno = REGNO (reg);
+  const hash_set<use_info *> vl_uses = get_all_real_uses (insn, regno);
+  for (use_info *use : vl_uses)
+    {
+      gcc_assert (use->insn ()->is_real ());
+      rtx_insn *rinsn = use->insn ()->rtl ();
+      if (!has_vl_op (rinsn)
+	  || count_regno_occurrences (rinsn, regno) != 1)
+	return false;
+
+      vsetvl_info info = vsetvl_info (use->insn ());
+
+      if (!info.has_nonvlmax_reg_avl ())
+	return false;
+
+      rtx avl = info.get_avl ();
+      if (avl && REG_P (avl) && regno == REGNO (avl))
+	return true;
+    }
+  return false;
+}
+
 /* Demand system is the RVV-based VSETVL info analysis tools wrapper.
    It defines compatible rules for SEW/LMUL, POLICY and AVL.
    Also, it provides 3 interfaces available_p, compatible_p and
@@ -2797,8 +2836,18 @@  pre_vsetvl::fuse_local_vsetvl_info ()
 		     64 into 32.  */
 		  prev_info.set_max_sew (
 		    MIN (prev_info.get_max_sew (), curr_info.get_max_sew ()));
-		  if (!curr_info.vl_used_by_non_rvv_insn_p ()
-		      && vsetvl_insn_p (curr_info.get_insn ()->rtl ()))
+
+		  /* If we fuse and the current, to be deleted vsetvl has uses
+		     of its VL as AVL operand in other vsetvls those will be
+		     orphaned.  Therefore only delete if that's not the case.
+		     */
+		  rtx cur_dest = curr_info.has_vl ()
+		    ? curr_info.get_vl ()
+		    : NULL_RTX;
+
+		  if (vsetvl_insn_p (curr_info.get_insn ()->rtl ())
+		      && !reg_single_use_in_avl (curr_info.get_insn (),
+						 cur_dest))
 		    m_delete_list.safe_push (curr_info);
 
 		  if (curr_info.get_read_vl_insn ())
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr119115.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr119115.c
new file mode 100644
index 00000000000..ac8a70e9edd
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr119115.c
@@ -0,0 +1,59 @@ 
+/* { dg-do run } */
+/* { dg-require-effective-target rv64 } */
+/* { dg-require-effective-target rvv_zvl256b_ok } */
+/* { dg-additional-options "-march=rv64gcv_zvl256b -mabi=lp64d -O3 -fsigned-char -fwrapv -mrvv-vector-bits=zvl" } */
+
+short a[4][14][14];
+void
+b (short d, short e, _Bool f, short g, int h, int i, char j, int k, int l,
+   short m[][14], char n[], unsigned short o[][14][14], short p[])
+{
+  for (unsigned q = 0; q < (char) i; q++)
+    for (unsigned char r = 0; r < (char) (-1748723647U * m[q][q]) - 20; r++)
+      a[q][q][r] = p[q] || o[q][q][r];
+}
+
+__attribute__ ((noipa))
+void
+check (long long val)
+{
+  if (val != 0)
+    __builtin_abort ();
+}
+
+long long seed;
+short d;
+short e;
+_Bool f;
+short g;
+int h;
+int i = 15963650;
+char j;
+int k;
+int l;
+short m[4][14];
+char n[4];
+unsigned short o[4][14][14];
+short p[4];
+
+int
+main ()
+{
+  for (int q = 0; q < 4; ++q)
+    {
+      p[q] = 4084;
+      for (int r = 0; r < 4; ++r)
+	{
+	  m[q][r] = 24482;
+	  for (int u = 0; u < 4; ++u)
+	    a[q][r][u] = 81;
+	}
+    }
+  b (d, e, f, g, h, i, j, k, l, m, n, o, p);
+  for (int q = 0; q < 4; ++q)
+    for (int r = 0; r < 4; ++r)
+      for (int u = 0; u < 14; ++u)
+	seed ^= a[q][r][u] + (seed >> 2);
+
+  check (seed);
+}