diff mbox series

RISC-V: fix vsetvli pass testsuite failure [PR/112447]

Message ID 20231115064107.2151843-1-vineetg@rivosinc.com
State New
Headers show
Series RISC-V: fix vsetvli pass testsuite failure [PR/112447] | expand

Commit Message

Vineet Gupta Nov. 15, 2023, 6:41 a.m. UTC
From: Juzhe-Zhong <juzhe.zhong@rivai.ai>

Fixes: f0e28d8c1371 ("RISC-V: Fix failed hoist in LICM of vmv.v.x instruction")

Since above commit, we have the following failure:

  FAIL: gcc.c-torture/execute/memset-3.c   -O3 -g  execution test

The issue was not the commit but it unravled an issue in vsetvli pass.
Here's Juzhe's analysis:

We have 2 types of global vsetvls insertion.
One is earliest fusion of each end of the block.
The other is LCM suggested edge vsetvls.

So before this patch, insertion as follows:

|  (insn 2817 2820 2818 361 (set (reg:SI 67 vtype)
|        (unspec:SI [
|                (const_int 8 [0x8])
|                (const_int 7 [0x7])
|                (const_int 1 [0x1]) repeated x2
|            ] UNSPEC_VSETVL)) 1708 {vsetvl_vtype_change_only}
|     (nil))
|  (insn 2818 2817 999 361 (set (reg:SI 67 vtype)
|        (unspec:SI [
|                (const_int 32 [0x20])
|                (const_int 1 [0x1]) repeated x3
|            ] UNSPEC_VSETVL)) 1708 {vsetvl_vtype_change_only}
|     (nil))

After this patch:

|  (insn 2817 2820 2819 361 (set (reg:SI 67 vtype)
|        (unspec:SI [
|                (const_int 32 [0x20])
|                (const_int 1 [0x1]) repeated x3
|            ] UNSPEC_VSETVL)) 1708 {vsetvl_vtype_change_only}
|     (nil))
|  (insn 2819 2817 999 361 (set (reg:SI 67 vtype)
|        (unspec:SI [
|                (const_int 8 [0x8])
|                (const_int 7 [0x7])
|                (const_int 1 [0x1]) repeated x2
|            ] UNSPEC_VSETVL)) 1708 {vsetvl_vtype_change_only}
|     (nil))

The original insertion order is incorrect.

We should first insert earliest fusion since it is the vsetvls information
already there which was seen by later LCM. We just delay the insertion.
So it should be come before the LCM suggested insertion.

Commit contientent to pre-commit CI Testing feedback.

	PR target/112447

Co-developed-by: Vineet Gupta <vineetg@rivosinc.com>

gcc/ChangeLog:
	* config/riscv/riscv-vsetvl.cc (pre_vsetvl::emit_vsetvl): Insert
	local vsetvl info before LCM suggested one.

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
 gcc/config/riscv/riscv-vsetvl.cc | 70 ++++++++++++++++----------------
 1 file changed, 35 insertions(+), 35 deletions(-)

Comments

钟居哲 Nov. 15, 2023, 6:43 a.m. UTC | #1
LGTM.  You can commit it. 
Thanks.



juzhe.zhong@rivai.ai
 
From: Vineet Gupta
Date: 2023-11-15 14:41
To: gcc-patches
CC: Jeff Law; Robin Dapp; gnu-toolchain; Patrick O'Neill; Juzhe-Zhong; Vineet Gupta
Subject: [PATCH] RISC-V: fix vsetvli pass testsuite failure [PR/112447]
From: Juzhe-Zhong <juzhe.zhong@rivai.ai>
 
Fixes: f0e28d8c1371 ("RISC-V: Fix failed hoist in LICM of vmv.v.x instruction")
 
Since above commit, we have the following failure:
 
  FAIL: gcc.c-torture/execute/memset-3.c   -O3 -g  execution test
 
The issue was not the commit but it unravled an issue in vsetvli pass.
Here's Juzhe's analysis:
 
We have 2 types of global vsetvls insertion.
One is earliest fusion of each end of the block.
The other is LCM suggested edge vsetvls.
 
So before this patch, insertion as follows:
 
|  (insn 2817 2820 2818 361 (set (reg:SI 67 vtype)
|        (unspec:SI [
|                (const_int 8 [0x8])
|                (const_int 7 [0x7])
|                (const_int 1 [0x1]) repeated x2
|            ] UNSPEC_VSETVL)) 1708 {vsetvl_vtype_change_only}
|     (nil))
|  (insn 2818 2817 999 361 (set (reg:SI 67 vtype)
|        (unspec:SI [
|                (const_int 32 [0x20])
|                (const_int 1 [0x1]) repeated x3
|            ] UNSPEC_VSETVL)) 1708 {vsetvl_vtype_change_only}
|     (nil))
 
After this patch:
 
|  (insn 2817 2820 2819 361 (set (reg:SI 67 vtype)
|        (unspec:SI [
|                (const_int 32 [0x20])
|                (const_int 1 [0x1]) repeated x3
|            ] UNSPEC_VSETVL)) 1708 {vsetvl_vtype_change_only}
|     (nil))
|  (insn 2819 2817 999 361 (set (reg:SI 67 vtype)
|        (unspec:SI [
|                (const_int 8 [0x8])
|                (const_int 7 [0x7])
|                (const_int 1 [0x1]) repeated x2
|            ] UNSPEC_VSETVL)) 1708 {vsetvl_vtype_change_only}
|     (nil))
 
The original insertion order is incorrect.
 
We should first insert earliest fusion since it is the vsetvls information
already there which was seen by later LCM. We just delay the insertion.
So it should be come before the LCM suggested insertion.
 
Commit contientent to pre-commit CI Testing feedback.
 
PR target/112447
 
Co-developed-by: Vineet Gupta <vineetg@rivosinc.com>
 
gcc/ChangeLog:
* config/riscv/riscv-vsetvl.cc (pre_vsetvl::emit_vsetvl): Insert
local vsetvl info before LCM suggested one.
 
Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
gcc/config/riscv/riscv-vsetvl.cc | 70 ++++++++++++++++----------------
1 file changed, 35 insertions(+), 35 deletions(-)
 
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 8466b5d019ea..74367ec8d8e9 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -3229,6 +3229,41 @@ pre_vsetvl::emit_vsetvl ()
       remove_vsetvl_insn (item);
     }
+  /* Insert vsetvl info that was not deleted after lift up.  */
+  for (const bb_info *bb : crtl->ssa->bbs ())
+    {
+      const vsetvl_block_info &block_info = get_block_info (bb);
+      if (!block_info.has_info ())
+ continue;
+
+      const vsetvl_info &footer_info = block_info.get_exit_info ();
+
+      if (footer_info.delete_p ())
+ continue;
+
+      edge eg;
+      edge_iterator eg_iterator;
+      FOR_EACH_EDGE (eg, eg_iterator, bb->cfg_bb ()->succs)
+ {
+   gcc_assert (!(eg->flags & EDGE_ABNORMAL));
+   if (dump_file)
+     {
+       fprintf (
+ dump_file,
+ "\n  Insert missed vsetvl info at edge(bb %u -> bb %u): ",
+ eg->src->index, eg->dest->index);
+       footer_info.dump (dump_file, "    ");
+     }
+   start_sequence ();
+   insert_vsetvl_insn (EMIT_DIRECT, footer_info);
+   rtx_insn *rinsn = get_insns ();
+   end_sequence ();
+   default_rtl_profile ();
+   insert_insn_on_edge (rinsn, eg);
+   need_commit = true;
+ }
+    }
+
   /* m_insert vsetvl as LCM suggest. */
   for (int ed = 0; ed < NUM_EDGES (m_edges); ed++)
     {
@@ -3267,41 +3302,6 @@ pre_vsetvl::emit_vsetvl ()
       insert_insn_on_edge (rinsn, eg);
     }
-  /* Insert vsetvl info that was not deleted after lift up.  */
-  for (const bb_info *bb : crtl->ssa->bbs ())
-    {
-      const vsetvl_block_info &block_info = get_block_info (bb);
-      if (!block_info.has_info ())
- continue;
-
-      const vsetvl_info &footer_info = block_info.get_exit_info ();
-
-      if (footer_info.delete_p ())
- continue;
-
-      edge eg;
-      edge_iterator eg_iterator;
-      FOR_EACH_EDGE (eg, eg_iterator, bb->cfg_bb ()->succs)
- {
-   gcc_assert (!(eg->flags & EDGE_ABNORMAL));
-   if (dump_file)
-     {
-       fprintf (
- dump_file,
- "\n  Insert missed vsetvl info at edge(bb %u -> bb %u): ",
- eg->src->index, eg->dest->index);
-       footer_info.dump (dump_file, "    ");
-     }
-   start_sequence ();
-   insert_vsetvl_insn (EMIT_DIRECT, footer_info);
-   rtx_insn *rinsn = get_insns ();
-   end_sequence ();
-   default_rtl_profile ();
-   insert_insn_on_edge (rinsn, eg);
-   need_commit = true;
- }
-    }
-
   if (need_commit)
     commit_edge_insertions ();
}
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 8466b5d019ea..74367ec8d8e9 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -3229,6 +3229,41 @@  pre_vsetvl::emit_vsetvl ()
       remove_vsetvl_insn (item);
     }
 
+  /* Insert vsetvl info that was not deleted after lift up.  */
+  for (const bb_info *bb : crtl->ssa->bbs ())
+    {
+      const vsetvl_block_info &block_info = get_block_info (bb);
+      if (!block_info.has_info ())
+	continue;
+
+      const vsetvl_info &footer_info = block_info.get_exit_info ();
+
+      if (footer_info.delete_p ())
+	continue;
+
+      edge eg;
+      edge_iterator eg_iterator;
+      FOR_EACH_EDGE (eg, eg_iterator, bb->cfg_bb ()->succs)
+	{
+	  gcc_assert (!(eg->flags & EDGE_ABNORMAL));
+	  if (dump_file)
+	    {
+	      fprintf (
+		dump_file,
+		"\n  Insert missed vsetvl info at edge(bb %u -> bb %u): ",
+		eg->src->index, eg->dest->index);
+	      footer_info.dump (dump_file, "    ");
+	    }
+	  start_sequence ();
+	  insert_vsetvl_insn (EMIT_DIRECT, footer_info);
+	  rtx_insn *rinsn = get_insns ();
+	  end_sequence ();
+	  default_rtl_profile ();
+	  insert_insn_on_edge (rinsn, eg);
+	  need_commit = true;
+	}
+    }
+
   /* m_insert vsetvl as LCM suggest. */
   for (int ed = 0; ed < NUM_EDGES (m_edges); ed++)
     {
@@ -3267,41 +3302,6 @@  pre_vsetvl::emit_vsetvl ()
       insert_insn_on_edge (rinsn, eg);
     }
 
-  /* Insert vsetvl info that was not deleted after lift up.  */
-  for (const bb_info *bb : crtl->ssa->bbs ())
-    {
-      const vsetvl_block_info &block_info = get_block_info (bb);
-      if (!block_info.has_info ())
-	continue;
-
-      const vsetvl_info &footer_info = block_info.get_exit_info ();
-
-      if (footer_info.delete_p ())
-	continue;
-
-      edge eg;
-      edge_iterator eg_iterator;
-      FOR_EACH_EDGE (eg, eg_iterator, bb->cfg_bb ()->succs)
-	{
-	  gcc_assert (!(eg->flags & EDGE_ABNORMAL));
-	  if (dump_file)
-	    {
-	      fprintf (
-		dump_file,
-		"\n  Insert missed vsetvl info at edge(bb %u -> bb %u): ",
-		eg->src->index, eg->dest->index);
-	      footer_info.dump (dump_file, "    ");
-	    }
-	  start_sequence ();
-	  insert_vsetvl_insn (EMIT_DIRECT, footer_info);
-	  rtx_insn *rinsn = get_insns ();
-	  end_sequence ();
-	  default_rtl_profile ();
-	  insert_insn_on_edge (rinsn, eg);
-	  need_commit = true;
-	}
-    }
-
   if (need_commit)
     commit_edge_insertions ();
 }