Re: FW: [PATCH 2/2] RISC-V: Eliminate latter vsetvl when fused

[钟居哲]

I see the codegen is incorrect before this patch:

foo:
        vsetvli a5,a0,e16,m1,ta,ma
        vmv.x.s a4,v8
        vsetvli a5,a0,e8,mf2,ta,ma ---> wrong VTYPE
        vadd.vx v9,v8,a4
        vsetvli zero,a5,e16,m1,ta,ma
        vadd.vv v8,v9,v8
        ret

Could you show me what the codegen looks like after this patch ?
I would be expecting the codegen become:

foo:
        vsetvli a5,a0,e16,m1,ta,ma
        vmv.x.s a4,v8
        vadd.vx v9,v8,a4
        vsetvli zero,a5,e16,m1,ta,ma
        vadd.vv v8,v9,v8
        ret
Or:
foo:
        vsetvli zero,a0,e16,m1,ta,ma
        vmv.x.s a4,v8
        vadd.vx v9,v8,a4
        vadd.vv v8,v9,v8
        ret
are both correct.

Also, I think it's better add assembly check in the testcase in stead of just 
adding "Eliminate insn" "vsetvl"

Thanks.


juzhe.zh...@rivai.ai
 
From: Li, Pan2
Date: 2024-09-11 19:28
To: juzhe.zh...@rivai.ai
Subject: FW: [PATCH 2/2] RISC-V: Eliminate latter vsetvl when fused
FYI
 
-----Original Message-----
From: Bohan Lei <garth...@linux.alibaba.com> 
Sent: Wednesday, September 11, 2024 5:13 PM
To: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH 2/2] RISC-V: Eliminate latter vsetvl when fused
 
The current vsetvl pass eliminates a vsetvl instruction when the previous
info is "available," but does not when "compatible."  This can lead to not
only redundancy, but also incorrect behaviors when the previous info happens
to be compatible with a later vector instruction, which ends of using the
vsetvl info that should have been eliminated, as is shown in the testcase.
This patch eliminates the vsetvl when the previous info is "compatible."
 
gcc/ChangeLog:
 
* config/riscv/riscv-vsetvl.cc (pre_vsetvl::fuse_local_vsetvl_info):
Delete vsetvl insn when `prev_info` is compatible
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/vsetvl/vsetvl_bug-4.c: New test.
---
gcc/config/riscv/riscv-vsetvl.cc               |  3 +++
.../gcc.target/riscv/rvv/vsetvl/vsetvl_bug-4.c | 18 ++++++++++++++++++
2 files changed, 21 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_bug-4.c
 
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index ce831685439..030ffbe2ebb 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -2796,6 +2796,9 @@ pre_vsetvl::fuse_local_vsetvl_info ()
      curr_info.dump (dump_file, "        ");
    }
  m_dem.merge (prev_info, curr_info);
+   if (!curr_info.vl_used_by_non_rvv_insn_p ()
+       && vsetvl_insn_p (curr_info.get_insn ()->rtl ()))
+     m_delete_list.safe_push (curr_info);
  if (curr_info.get_read_vl_insn ())
    prev_info.set_read_vl_insn (curr_info.get_read_vl_insn ());
  if (dump_file && (dump_flags & TDF_DETAILS))
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_bug-4.c 
b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_bug-4.c
new file mode 100644
index 00000000000..faa0c8073d3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_bug-4.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O2 -fno-schedule-insns 
-fdump-rtl-vsetvl-details" } */
+
+#include <riscv_vector.h>
+
+vuint16m1_t
+foo (vuint16m1_t a, vuint16m1_t b, size_t avl)
+{
+  size_t vl;
+  vuint16m1_t ret;
+  uint16_t c = __riscv_vmv_x_s_u16m1_u16(a);
+  vl = __riscv_vsetvl_e8mf2 (avl);
+  ret = __riscv_vadd_vx_u16m1 (a, c, avl);
+  ret = __riscv_vadd_vv_u16m1 (ret, a, vl);
+  return ret;
+}
+
+/* { dg-final { scan-rtl-dump "Eliminate insn" "vsetvl" } }  */
-- 
2.17.1