diff mbox series

RISC-V: Fix VSETVL PASS bug

Message ID 20231206130351.2573949-1-juzhe.zhong@rivai.ai
State Unresolved
Headers
Series RISC-V: Fix VSETVL PASS bug |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

juzhe.zhong@rivai.ai Dec. 6, 2023, 1:03 p.m. UTC 
  As PR112855 mentioned, the VSETVL PASS insert vsetvli in unexpected location.

Due to 2 reasons:
1. incorrect transparant computation LCM data. We need to check VL operand defs and uses.
2. incorrect fusion of unrelated edge which is the edge never reach the vsetvl expression.

	PR target/112855

gcc/ChangeLog:

	* config/riscv/riscv-vsetvl.cc (pre_vsetvl::compute_lcm_local_properties): Fix transparant LCM data.
	(pre_vsetvl::earliest_fuse_vsetvl_info): Disable earliest fusion for unrelated edge.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/autovec/pr112855.c: New test.

---
 gcc/config/riscv/riscv-vsetvl.cc              | 63 ++++++++++++++++++-
 .../gcc.target/riscv/rvv/autovec/pr112855.c   | 26 ++++++++
 2 files changed, 86 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112855.c

Comments

Robin Dapp Dec. 6, 2023, 2:07 p.m. UTC | #1 
LGTM.

+	  /* Don't perform earliest fusion on unrelated edge.  */
+	  if (bitmap_count_bits (e) != 1)
+	    continue;

This could still use a comment why e is "unrelated" in that case
(no v2 needed).

Regards
 Robin
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 484a8b3a514..f0dd43bece7 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -2034,7 +2034,7 @@  private:
     gcc_unreachable ();
   }
 
-  bool anticpatable_exp_p (const vsetvl_info &header_info)
+  bool anticipated_exp_p (const vsetvl_info &header_info)
   {
     if (!header_info.has_nonvlmax_reg_avl () && !header_info.has_vl ())
       return true;
@@ -2645,7 +2645,7 @@  pre_vsetvl::compute_lcm_local_properties ()
 		    }
 		}
 
-	      for (const insn_info *insn : bb->real_nondebug_insns ())
+	      for (insn_info *insn : bb->real_nondebug_insns ())
 		{
 		  if (info.has_nonvlmax_reg_avl ()
 		      && find_access (insn->defs (), REGNO (info.get_avl ())))
@@ -2653,6 +2653,59 @@  pre_vsetvl::compute_lcm_local_properties ()
 		      bitmap_clear_bit (m_transp[bb_index], i);
 		      break;
 		    }
+
+		  if (info.has_vl ()
+		      && reg_mentioned_p (info.get_vl (), insn->rtl ()))
+		    {
+		      if (find_access (insn->defs (), REGNO (info.get_vl ())))
+			/* We can't fuse vsetvl into the blocks that modify the
+			   VL operand since successors of such blocks will need
+			   the value of those blocks are defining.
+
+					  bb 4: def a5
+					  /   \
+				  bb 5:use a5  bb 6:vsetvl a5, 5
+
+			   The example above shows that we can't fuse vsetvl
+			   from bb 6 into bb 4 since the successor bb 5 is using
+			   the value defined in bb 4.  */
+			;
+		      else
+			{
+			  /* We can't fuse vsetvl into the blocks that use the
+			     VL operand which has different value from the
+			     vsetvl info.
+
+					    bb 4: def a5
+					      |
+					    bb 5: use a5
+					      |
+					    bb 6: def a5
+					      |
+					    bb 7: use a5
+
+			     The example above shows that we can't fuse vsetvl
+			     from bb 6 into bb 5 since their value is different.
+			   */
+			  resource_info resource
+			    = full_register (REGNO (info.get_vl ()));
+			  def_lookup dl = crtl->ssa->find_def (resource, insn);
+			  def_info *def
+			    = dl.matching_set_or_last_def_of_prev_group ();
+			  gcc_assert (def);
+			  insn_info *def_insn = extract_single_source (
+			    dyn_cast<set_info *> (def));
+			  if (def_insn && vsetvl_insn_p (def_insn->rtl ()))
+			    {
+			      vsetvl_info def_info = vsetvl_info (def_insn);
+			      if (m_dem.compatible_p (def_info, info))
+				continue;
+			    }
+			}
+
+		      bitmap_clear_bit (m_transp[bb_index], i);
+		      break;
+		    }
 		}
 	    }
 
@@ -2663,7 +2716,7 @@  pre_vsetvl::compute_lcm_local_properties ()
       vsetvl_info &footer_info = block_info.get_exit_info ();
 
       if (header_info.valid_p ()
-	  && (anticpatable_exp_p (header_info) || block_info.full_available))
+	  && (anticipated_exp_p (header_info) || block_info.full_available))
 	bitmap_set_bit (m_antloc[bb_index],
 			get_expr_index (m_exprs, header_info));
 
@@ -2920,6 +2973,10 @@  pre_vsetvl::earliest_fuse_vsetvl_info ()
 	      || eg->dest == EXIT_BLOCK_PTR_FOR_FN (cfun))
 	    continue;
 
+	  /* Don't perform earliest fusion on unrelated edge.  */
+	  if (bitmap_count_bits (e) != 1)
+	    continue;
+
 	  vsetvl_block_info &src_block_info = get_block_info (eg->src);
 	  vsetvl_block_info &dest_block_info = get_block_info (eg->dest);
 
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112855.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112855.c
new file mode 100644
index 00000000000..f1fa6693d2d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr112855.c
@@ -0,0 +1,26 @@ 
+/* { dg-do run } */
+/* { dg-options "-O3" } */
+/* { dg-require-effective-target rv64 } */
+/* { dg-require-effective-target riscv_v } */
+
+#include <assert.h>
+int a;
+int b = 100;
+int c[25];
+int d;
+int main() {
+  int e;
+  d = 0;
+  for (; d < 5; d++) {
+    e = 0;
+    for (; e < 5; e++)
+      c[d * 5 + e] = 0;
+  }
+  if (b)
+    if (a)
+      for (;;)
+        ;
+  b++;
+  int volatile f = *c;
+  assert(b == 101);
+}