diff mbox series

RISC-V: Remove vsetvl_pre bogus instructions in VSETVL PASS

Message ID 20240201122448.182111-1-juzhe.zhong@rivai.ai
State New
Headers show
Series RISC-V: Remove vsetvl_pre bogus instructions in VSETVL PASS | expand

Commit Message

juzhe.zhong@rivai.ai Feb. 1, 2024, 12:24 p.m. UTC 
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

Kito Cheng Feb. 1, 2024, 12:44 p.m. UTC | #1 
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
>
Robin Dapp Feb. 1, 2024, 12:45 p.m. UTC | #2 
> +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
diff mbox series

Patch

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" } } } }  */