RISC-V: Remove vsetvl_pre bogus instructions in VSETVL PASS
Checks
Commit Message
I realize there is a RTL regression between GCC-14 and GCC-13.
https://godbolt.org/z/Ga7K6MqaT
GCC-14:
(insn 9 13 31 2 (set (reg:DI 15 a5 [138])
(unspec:DI [
(const_int 64 [0x40])
] UNSPEC_VLMAX)) "/app/example.c":5:15 2566 {vlmax_avldi}
(expr_list:REG_EQUIV (unspec:DI [
(const_int 64 [0x40])
] UNSPEC_VLMAX)
(nil)))
(insn 31 9 10 2 (parallel [
(set (reg:DI 15 a5 [138])
(unspec:DI [
(reg:DI 0 zero)
(const_int 32 [0x20])
(const_int 7 [0x7])
(const_int 1 [0x1]) repeated x2
] UNSPEC_VSETVL))
(set (reg:SI 66 vl)
(unspec:SI [
(reg:DI 0 zero)
(const_int 32 [0x20])
(const_int 7 [0x7])
] UNSPEC_VSETVL))
(set (reg:SI 67 vtype)
(unspec:SI [
(const_int 32 [0x20])
(const_int 7 [0x7])
(const_int 1 [0x1]) repeated x2
] UNSPEC_VSETVL))
]) "/app/example.c":5:15 3281 {vsetvldi}
(nil))
GCC-13:
(insn 10 7 26 2 (set (reg/f:DI 11 a1 [139])
(plus:DI (reg:DI 11 a1 [142])
(const_int 800 [0x320]))) "/app/example.c":6:32 5 {adddi3}
(nil))
(insn 26 10 9 2 (parallel [
(set (reg:DI 15 a5)
(unspec:DI [
(reg:DI 0 zero)
(const_int 32 [0x20])
(const_int 7 [0x7])
(const_int 1 [0x1]) repeated x2
] UNSPEC_VSETVL))
(set (reg:SI 66 vl)
(unspec:SI [
(reg:DI 0 zero)
(const_int 32 [0x20])
(const_int 7 [0x7])
] UNSPEC_VSETVL))
(set (reg:SI 67 vtype)
(unspec:SI [
(const_int 32 [0x20])
(const_int 7 [0x7])
(const_int 1 [0x1]) repeated x2
] UNSPEC_VSETVL))
]) "/app/example.c":5:15 792 {vsetvldi}
(nil))
GCC-13 doesn't have:
(insn 9 13 31 2 (set (reg:DI 15 a5 [138])
(unspec:DI [
(const_int 64 [0x40])
] UNSPEC_VLMAX)) "/app/example.c":5:15 2566 {vlmax_avldi}
(expr_list:REG_EQUIV (unspec:DI [
(const_int 64 [0x40])
] UNSPEC_VLMAX)
(nil)))
vsetvl_pre doesn't emit any assembler which is just used for occupying scalar register.
It should be removed in VSETVL PASS.
Tested on both RV32 and RV64 no regression.
gcc/ChangeLog:
* config/riscv/riscv-vsetvl.cc (vsetvl_pre_insn_p): New function.
(pre_vsetvl::cleaup): Remove vsetvl_pre.
(pre_vsetvl::remove_vsetvl_pre_insns): New function.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/vsetvl/vsetvl_pre-1.c: New test.
---
gcc/config/riscv/riscv-vsetvl.cc | 64 +++++++++++++++++++
.../riscv/rvv/vsetvl/vsetvl_pre-1.c | 12 ++++
2 files changed, 76 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_pre-1.c
Comments
LGTM
On Thu, Feb 1, 2024 at 8:25 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
>
> I realize there is a RTL regression between GCC-14 and GCC-13.
> https://godbolt.org/z/Ga7K6MqaT
>
> GCC-14:
> (insn 9 13 31 2 (set (reg:DI 15 a5 [138])
> (unspec:DI [
> (const_int 64 [0x40])
> ] UNSPEC_VLMAX)) "/app/example.c":5:15 2566 {vlmax_avldi}
> (expr_list:REG_EQUIV (unspec:DI [
> (const_int 64 [0x40])
> ] UNSPEC_VLMAX)
> (nil)))
> (insn 31 9 10 2 (parallel [
> (set (reg:DI 15 a5 [138])
> (unspec:DI [
> (reg:DI 0 zero)
> (const_int 32 [0x20])
> (const_int 7 [0x7])
> (const_int 1 [0x1]) repeated x2
> ] UNSPEC_VSETVL))
> (set (reg:SI 66 vl)
> (unspec:SI [
> (reg:DI 0 zero)
> (const_int 32 [0x20])
> (const_int 7 [0x7])
> ] UNSPEC_VSETVL))
> (set (reg:SI 67 vtype)
> (unspec:SI [
> (const_int 32 [0x20])
> (const_int 7 [0x7])
> (const_int 1 [0x1]) repeated x2
> ] UNSPEC_VSETVL))
> ]) "/app/example.c":5:15 3281 {vsetvldi}
> (nil))
>
> GCC-13:
> (insn 10 7 26 2 (set (reg/f:DI 11 a1 [139])
> (plus:DI (reg:DI 11 a1 [142])
> (const_int 800 [0x320]))) "/app/example.c":6:32 5 {adddi3}
> (nil))
> (insn 26 10 9 2 (parallel [
> (set (reg:DI 15 a5)
> (unspec:DI [
> (reg:DI 0 zero)
> (const_int 32 [0x20])
> (const_int 7 [0x7])
> (const_int 1 [0x1]) repeated x2
> ] UNSPEC_VSETVL))
> (set (reg:SI 66 vl)
> (unspec:SI [
> (reg:DI 0 zero)
> (const_int 32 [0x20])
> (const_int 7 [0x7])
> ] UNSPEC_VSETVL))
> (set (reg:SI 67 vtype)
> (unspec:SI [
> (const_int 32 [0x20])
> (const_int 7 [0x7])
> (const_int 1 [0x1]) repeated x2
> ] UNSPEC_VSETVL))
> ]) "/app/example.c":5:15 792 {vsetvldi}
> (nil))
>
> GCC-13 doesn't have:
> (insn 9 13 31 2 (set (reg:DI 15 a5 [138])
> (unspec:DI [
> (const_int 64 [0x40])
> ] UNSPEC_VLMAX)) "/app/example.c":5:15 2566 {vlmax_avldi}
> (expr_list:REG_EQUIV (unspec:DI [
> (const_int 64 [0x40])
> ] UNSPEC_VLMAX)
> (nil)))
>
> vsetvl_pre doesn't emit any assembler which is just used for occupying scalar register.
> It should be removed in VSETVL PASS.
>
> Tested on both RV32 and RV64 no regression.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-vsetvl.cc (vsetvl_pre_insn_p): New function.
> (pre_vsetvl::cleaup): Remove vsetvl_pre.
> (pre_vsetvl::remove_vsetvl_pre_insns): New function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/vsetvl/vsetvl_pre-1.c: New test.
>
> ---
> gcc/config/riscv/riscv-vsetvl.cc | 64 +++++++++++++++++++
> .../riscv/rvv/vsetvl/vsetvl_pre-1.c | 12 ++++
> 2 files changed, 76 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_pre-1.c
>
> diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
> index 28b7534d970..4732d4fc77f 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -315,6 +315,48 @@ vsetvl_insn_p (rtx_insn *rinsn)
> || INSN_CODE (rinsn) == CODE_FOR_vsetvlsi);
> }
>
> +/* Return true if it is the bogus vsetvl_pre instruction:
> +
> + (define_insn "@vlmax_avl<mode>"
> + [(set (match_operand:P 0 "register_operand" "=r")
> + (unspec:P [(match_operand:P 1 "const_int_operand" "i")] UNSPEC_VLMAX))]
> + "TARGET_VECTOR"
> + ""
> + [(set_attr "type" "vsetvl_pre")])
> +
> + As described above, it's the bogus instruction which doesn't any assembler
> + and should be removed eventually. It's used for occupying a scalar register
> + for VLMAX avl RVV instruction before register allocation.
> +
> + Before RA:
> +
> + ...
> + vsetvl_pre (set r136)
> + vadd.vv (use r136 with VLMAX avl)
> + ...
> +
> + After RA:
> +
> + ...
> + vsetvl_pre (set a5)
> + vadd.vv (use r136 with VLMAX avl)
> + ...
> +
> + VSETVL PASS:
> +
> + ...
> + vsetvl_pre (set a5) ---> removed.
> + vsetvl a5,zero,... ---> Inserted.
> + vadd.vv
> + ...
> +*/
> +static bool
> +vsetvl_pre_insn_p (rtx_insn *rinsn)
> +{
> + return recog_memoized (rinsn) >= 0
> + && get_attr_type (rinsn) == TYPE_VSETVL_PRE;
> +}
> +
> /* Return true if it is vsetvl zero, rs1. */
> static bool
> vsetvl_discard_result_insn_p (rtx_insn *rinsn)
> @@ -2376,6 +2418,7 @@ public:
> void cleaup ();
> void remove_avl_operand ();
> void remove_unused_dest_operand ();
> + void remove_vsetvl_pre_insns ();
>
> void dump (FILE *file, const char *title) const
> {
> @@ -3332,6 +3375,7 @@ pre_vsetvl::cleaup ()
> {
> remove_avl_operand ();
> remove_unused_dest_operand ();
> + remove_vsetvl_pre_insns ();
> }
>
> void
> @@ -3399,6 +3443,26 @@ pre_vsetvl::remove_unused_dest_operand ()
> }
> }
>
> +/* Remove all bogus vsetvl_pre instructions. */
> +void
> +pre_vsetvl::remove_vsetvl_pre_insns ()
> +{
> + basic_block cfg_bb;
> + rtx_insn *rinsn;
> + FOR_ALL_BB_FN (cfg_bb, cfun)
> + FOR_BB_INSNS (cfg_bb, rinsn)
> + if (NONDEBUG_INSN_P (rinsn) && vsetvl_pre_insn_p (rinsn))
> + {
> + if (dump_file)
> + {
> + fprintf (dump_file, " Eliminate vsetvl_pre insn %d:\n",
> + INSN_UID (rinsn));
> + print_rtl_single (dump_file, rinsn);
> + }
> + remove_insn (rinsn);
> + }
> +}
> +
> const pass_data pass_data_vsetvl = {
> RTL_PASS, /* type */
> "vsetvl", /* name */
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_pre-1.c b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_pre-1.c
> new file mode 100644
> index 00000000000..98eacc10161
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_pre-1.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "--param=riscv-autovec-preference=scalable -march=rv64gcv -mabi=lp64d -O3 -fdump-rtl-vsetvl-details" } */
> +
> +#include "riscv_vector.h"
> +void
> +foo (int *in, int *out)
> +{
> + vint32mf2_t v = *(vint32mf2_t *) (in + 100);
> + *(vint32mf2_t *) (out + 200) = v;
> +}
> +
> +/* { dg-final { scan-rtl-dump "Eliminate vsetvl_pre" "vsetvl" { target { no-opts "-O0" } } } } */
> --
> 2.36.3
>
> +static bool
> +vsetvl_pre_insn_p (rtx_insn *rinsn)
> +{
> + return recog_memoized (rinsn) >= 0
> + && get_attr_type (rinsn) == TYPE_VSETVL_PRE;
> +}
Indent looks off on my screen. Can you check?
Apart from that LGTM (no need for v2 of course).
Regards
Robin
@@ -315,6 +315,48 @@ vsetvl_insn_p (rtx_insn *rinsn)
|| INSN_CODE (rinsn) == CODE_FOR_vsetvlsi);
}
+/* Return true if it is the bogus vsetvl_pre instruction:
+
+ (define_insn "@vlmax_avl<mode>"
+ [(set (match_operand:P 0 "register_operand" "=r")
+ (unspec:P [(match_operand:P 1 "const_int_operand" "i")] UNSPEC_VLMAX))]
+ "TARGET_VECTOR"
+ ""
+ [(set_attr "type" "vsetvl_pre")])
+
+ As described above, it's the bogus instruction which doesn't any assembler
+ and should be removed eventually. It's used for occupying a scalar register
+ for VLMAX avl RVV instruction before register allocation.
+
+ Before RA:
+
+ ...
+ vsetvl_pre (set r136)
+ vadd.vv (use r136 with VLMAX avl)
+ ...
+
+ After RA:
+
+ ...
+ vsetvl_pre (set a5)
+ vadd.vv (use r136 with VLMAX avl)
+ ...
+
+ VSETVL PASS:
+
+ ...
+ vsetvl_pre (set a5) ---> removed.
+ vsetvl a5,zero,... ---> Inserted.
+ vadd.vv
+ ...
+*/
+static bool
+vsetvl_pre_insn_p (rtx_insn *rinsn)
+{
+ return recog_memoized (rinsn) >= 0
+ && get_attr_type (rinsn) == TYPE_VSETVL_PRE;
+}
+
/* Return true if it is vsetvl zero, rs1. */
static bool
vsetvl_discard_result_insn_p (rtx_insn *rinsn)
@@ -2376,6 +2418,7 @@ public:
void cleaup ();
void remove_avl_operand ();
void remove_unused_dest_operand ();
+ void remove_vsetvl_pre_insns ();
void dump (FILE *file, const char *title) const
{
@@ -3332,6 +3375,7 @@ pre_vsetvl::cleaup ()
{
remove_avl_operand ();
remove_unused_dest_operand ();
+ remove_vsetvl_pre_insns ();
}
void
@@ -3399,6 +3443,26 @@ pre_vsetvl::remove_unused_dest_operand ()
}
}
+/* Remove all bogus vsetvl_pre instructions. */
+void
+pre_vsetvl::remove_vsetvl_pre_insns ()
+{
+ basic_block cfg_bb;
+ rtx_insn *rinsn;
+ FOR_ALL_BB_FN (cfg_bb, cfun)
+ FOR_BB_INSNS (cfg_bb, rinsn)
+ if (NONDEBUG_INSN_P (rinsn) && vsetvl_pre_insn_p (rinsn))
+ {
+ if (dump_file)
+ {
+ fprintf (dump_file, " Eliminate vsetvl_pre insn %d:\n",
+ INSN_UID (rinsn));
+ print_rtl_single (dump_file, rinsn);
+ }
+ remove_insn (rinsn);
+ }
+}
+
const pass_data pass_data_vsetvl = {
RTL_PASS, /* type */
"vsetvl", /* name */
new file mode 100644
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "--param=riscv-autovec-preference=scalable -march=rv64gcv -mabi=lp64d -O3 -fdump-rtl-vsetvl-details" } */
+
+#include "riscv_vector.h"
+void
+foo (int *in, int *out)
+{
+ vint32mf2_t v = *(vint32mf2_t *) (in + 100);
+ *(vint32mf2_t *) (out + 200) = v;
+}
+
+/* { dg-final { scan-rtl-dump "Eliminate vsetvl_pre" "vsetvl" { target { no-opts "-O0" } } } } */