public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Expand VLMAX scalar move in reduction
@ 2024-02-02  1:56 Juzhe-Zhong
  2024-02-02  9:50 ` Kito Cheng
  2024-02-05  3:26 ` Jeff Law
  0 siblings, 2 replies; 7+ messages in thread
From: Juzhe-Zhong @ 2024-02-02  1:56 UTC (permalink / raw)
  To: gcc-patches; +Cc: kito.cheng, kito.cheng, jeffreyalaw, rdapp.gcc, Juzhe-Zhong

This patch fixes the following:

        vsetvli a5,a1,e32,m1,tu,ma
        slli    a4,a5,2
        sub     a1,a1,a5
        vle32.v v2,0(a0)
        add     a0,a0,a4
        vadd.vv v1,v2,v1
        bne     a1,zero,.L3
        vsetivli        zero,1,e32,m1,ta,ma
        vmv.s.x v2,zero
        vsetvli a5,zero,e32,m1,ta,ma              ---> Redundant vsetvl.
        vredsum.vs      v1,v1,v2
        vmv.x.s a0,v1
        ret

VSETVL PASS is able to fuse avl = 1 of scalar move and VLMAX avl of reduction.

However, this following RTL blocks the fusion in dependence analysis in VSETVL PASS:

(insn 49 24 50 5 (set (reg:RVVM1SI 98 v2 [148])
        (if_then_else:RVVM1SI (unspec:RVVMF32BI [
                    (const_vector:RVVMF32BI [
                            (const_int 1 [0x1])
                            repeat [
                                (const_int 0 [0])
                            ]
                        ])
                    (const_int 1 [0x1])
                    (const_int 2 [0x2]) repeated x2
                    (const_int 0 [0])
                    (reg:SI 66 vl)
                    (reg:SI 67 vtype)
                ] UNSPEC_VPREDICATE)
            (const_vector:RVVM1SI repeat [
                    (const_int 0 [0])
                ])
            (unspec:RVVM1SI [
                    (reg:DI 0 zero)
                ] UNSPEC_VUNDEF))) 3813 {*pred_broadcastrvvm1si_zero}
     (nil))
(insn 50 49 51 5 (set (reg:DI 15 a5 [151])                          ---->  It set a5, blocks the following VLMAX into the scalar move above.
        (unspec:DI [
                (const_int 32 [0x20])
            ] UNSPEC_VLMAX)) 2566 {vlmax_avldi}
     (expr_list:REG_EQUIV (unspec:DI [
                (const_int 32 [0x20])
            ] UNSPEC_VLMAX)
        (nil)))
(insn 51 50 52 5 (set (reg:RVVM1SI 97 v1 [150])
        (unspec:RVVM1SI [
                (unspec:RVVMF32BI [
                        (const_vector:RVVMF32BI repeat [
                                (const_int 1 [0x1])
                            ])
                        (reg:DI 15 a5 [151])
                        (const_int 2 [0x2])
                        (const_int 1 [0x1])
                        (reg:SI 66 vl)
                        (reg:SI 67 vtype)
                    ] UNSPEC_VPREDICATE)
                (unspec:RVVM1SI [
                        (reg:RVVM1SI 97 v1 [orig:134 vect_result_14.6 ] [134])
                        (reg:RVVM1SI 98 v2 [148])
                    ] UNSPEC_REDUC_SUM)
                (unspec:RVVM1SI [
                        (reg:DI 0 zero)
                    ] UNSPEC_VUNDEF)
            ] UNSPEC_REDUC)) 17541 {pred_redsumrvvm1si}
     (expr_list:REG_DEAD (reg:RVVM1SI 98 v2 [148])
        (expr_list:REG_DEAD (reg:SI 66 vl)
            (expr_list:REG_DEAD (reg:DI 15 a5 [151])
                (expr_list:REG_DEAD (reg:DI 0 zero)
                    (nil))))))

Such situation can only happen on auto-vectorization, never happen on intrinsic codes.
Since the reduction is passed VLMAX AVL, it should be more natural to pass VLMAX to the scalar move which initial the value of the reduction.

After this patch:

	vsetvli	a5,a1,e32,m1,tu,ma
	slli	a4,a5,2
	sub	a1,a1,a5
	vle32.v	v2,0(a0)
	add	a0,a0,a4
	vadd.vv	v1,v2,v1
	bne	a1,zero,.L3
	vsetvli	a5,zero,e32,m1,ta,ma
	vmv.s.x	v2,zero
	vredsum.vs	v1,v1,v2
	vmv.x.s	a0,v1
        ret

Tested on both RV32/RV64 no regression.

	PR target/113697

gcc/ChangeLog:

	* config/riscv/riscv-v.cc (expand_reduction): Pass VLMAX avl to scalar move.

gcc/testsuite/ChangeLog:

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

---
 gcc/config/riscv/riscv-v.cc                        | 12 +++++++-----
 .../gcc.target/riscv/rvv/autovec/pr113697.c        | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113697.c

diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index 4bacb7fea45..0cfbd21ce6f 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -4151,13 +4151,15 @@ expand_reduction (unsigned unspec, unsigned insn_flags, rtx *ops, rtx init)
 
   rtx m1_tmp = gen_reg_rtx (m1_mode);
   rtx scalar_move_ops[] = {m1_tmp, init};
-  emit_nonvlmax_insn (code_for_pred_broadcast (m1_mode), SCALAR_MOVE_OP,
-		      scalar_move_ops,
-		      need_mask_operand_p (insn_flags) ? ops[3]
-						       : CONST1_RTX (Pmode));
+  insn_code icode = code_for_pred_broadcast (m1_mode);
+  if (need_mask_operand_p (insn_flags))
+    emit_nonvlmax_insn (icode, SCALAR_MOVE_OP, scalar_move_ops, ops[3]);
+  else
+    emit_vlmax_insn (icode, SCALAR_MOVE_OP, scalar_move_ops);
+
   rtx m1_tmp2 = gen_reg_rtx (m1_mode);
   rtx reduc_ops[] = {m1_tmp2, vector_src, m1_tmp};
-  insn_code icode = code_for_pred (unspec, vmode);
+  icode = code_for_pred (unspec, vmode);
 
   if (need_mask_operand_p (insn_flags))
     {
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113697.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113697.c
new file mode 100644
index 00000000000..588b86c7e6c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113697.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -march=rv64gcv -mabi=lp64d -fno-schedule-insns" } */
+
+int
+foo (int *__restrict a, int n)
+{
+  int result = 0;
+  for (int i = 0; i < n; i++)
+    result += a[i];
+  return result;
+}
+
+/* { dg-final { scan-assembler-times {vsetvli} 3 } } */
+/* { dg-final { scan-assembler-not {vsetivli} } } */
-- 
2.36.3


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] RISC-V: Expand VLMAX scalar move in reduction
  2024-02-02  1:56 [PATCH] RISC-V: Expand VLMAX scalar move in reduction Juzhe-Zhong
@ 2024-02-02  9:50 ` Kito Cheng
  2024-02-05  3:26 ` Jeff Law
  1 sibling, 0 replies; 7+ messages in thread
From: Kito Cheng @ 2024-02-02  9:50 UTC (permalink / raw)
  To: Juzhe-Zhong; +Cc: gcc-patches, kito.cheng, jeffreyalaw, rdapp.gcc

LGTM :)

On Fri, Feb 2, 2024 at 9:58 AM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
>
> This patch fixes the following:
>
>         vsetvli a5,a1,e32,m1,tu,ma
>         slli    a4,a5,2
>         sub     a1,a1,a5
>         vle32.v v2,0(a0)
>         add     a0,a0,a4
>         vadd.vv v1,v2,v1
>         bne     a1,zero,.L3
>         vsetivli        zero,1,e32,m1,ta,ma
>         vmv.s.x v2,zero
>         vsetvli a5,zero,e32,m1,ta,ma              ---> Redundant vsetvl.
>         vredsum.vs      v1,v1,v2
>         vmv.x.s a0,v1
>         ret
>
> VSETVL PASS is able to fuse avl = 1 of scalar move and VLMAX avl of reduction.
>
> However, this following RTL blocks the fusion in dependence analysis in VSETVL PASS:
>
> (insn 49 24 50 5 (set (reg:RVVM1SI 98 v2 [148])
>         (if_then_else:RVVM1SI (unspec:RVVMF32BI [
>                     (const_vector:RVVMF32BI [
>                             (const_int 1 [0x1])
>                             repeat [
>                                 (const_int 0 [0])
>                             ]
>                         ])
>                     (const_int 1 [0x1])
>                     (const_int 2 [0x2]) repeated x2
>                     (const_int 0 [0])
>                     (reg:SI 66 vl)
>                     (reg:SI 67 vtype)
>                 ] UNSPEC_VPREDICATE)
>             (const_vector:RVVM1SI repeat [
>                     (const_int 0 [0])
>                 ])
>             (unspec:RVVM1SI [
>                     (reg:DI 0 zero)
>                 ] UNSPEC_VUNDEF))) 3813 {*pred_broadcastrvvm1si_zero}
>      (nil))
> (insn 50 49 51 5 (set (reg:DI 15 a5 [151])                          ---->  It set a5, blocks the following VLMAX into the scalar move above.
>         (unspec:DI [
>                 (const_int 32 [0x20])
>             ] UNSPEC_VLMAX)) 2566 {vlmax_avldi}
>      (expr_list:REG_EQUIV (unspec:DI [
>                 (const_int 32 [0x20])
>             ] UNSPEC_VLMAX)
>         (nil)))
> (insn 51 50 52 5 (set (reg:RVVM1SI 97 v1 [150])
>         (unspec:RVVM1SI [
>                 (unspec:RVVMF32BI [
>                         (const_vector:RVVMF32BI repeat [
>                                 (const_int 1 [0x1])
>                             ])
>                         (reg:DI 15 a5 [151])
>                         (const_int 2 [0x2])
>                         (const_int 1 [0x1])
>                         (reg:SI 66 vl)
>                         (reg:SI 67 vtype)
>                     ] UNSPEC_VPREDICATE)
>                 (unspec:RVVM1SI [
>                         (reg:RVVM1SI 97 v1 [orig:134 vect_result_14.6 ] [134])
>                         (reg:RVVM1SI 98 v2 [148])
>                     ] UNSPEC_REDUC_SUM)
>                 (unspec:RVVM1SI [
>                         (reg:DI 0 zero)
>                     ] UNSPEC_VUNDEF)
>             ] UNSPEC_REDUC)) 17541 {pred_redsumrvvm1si}
>      (expr_list:REG_DEAD (reg:RVVM1SI 98 v2 [148])
>         (expr_list:REG_DEAD (reg:SI 66 vl)
>             (expr_list:REG_DEAD (reg:DI 15 a5 [151])
>                 (expr_list:REG_DEAD (reg:DI 0 zero)
>                     (nil))))))
>
> Such situation can only happen on auto-vectorization, never happen on intrinsic codes.
> Since the reduction is passed VLMAX AVL, it should be more natural to pass VLMAX to the scalar move which initial the value of the reduction.
>
> After this patch:
>
>         vsetvli a5,a1,e32,m1,tu,ma
>         slli    a4,a5,2
>         sub     a1,a1,a5
>         vle32.v v2,0(a0)
>         add     a0,a0,a4
>         vadd.vv v1,v2,v1
>         bne     a1,zero,.L3
>         vsetvli a5,zero,e32,m1,ta,ma
>         vmv.s.x v2,zero
>         vredsum.vs      v1,v1,v2
>         vmv.x.s a0,v1
>         ret
>
> Tested on both RV32/RV64 no regression.
>
>         PR target/113697
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv-v.cc (expand_reduction): Pass VLMAX avl to scalar move.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/autovec/pr113697.c: New test.
>
> ---
>  gcc/config/riscv/riscv-v.cc                        | 12 +++++++-----
>  .../gcc.target/riscv/rvv/autovec/pr113697.c        | 14 ++++++++++++++
>  2 files changed, 21 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113697.c
>
> diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
> index 4bacb7fea45..0cfbd21ce6f 100644
> --- a/gcc/config/riscv/riscv-v.cc
> +++ b/gcc/config/riscv/riscv-v.cc
> @@ -4151,13 +4151,15 @@ expand_reduction (unsigned unspec, unsigned insn_flags, rtx *ops, rtx init)
>
>    rtx m1_tmp = gen_reg_rtx (m1_mode);
>    rtx scalar_move_ops[] = {m1_tmp, init};
> -  emit_nonvlmax_insn (code_for_pred_broadcast (m1_mode), SCALAR_MOVE_OP,
> -                     scalar_move_ops,
> -                     need_mask_operand_p (insn_flags) ? ops[3]
> -                                                      : CONST1_RTX (Pmode));
> +  insn_code icode = code_for_pred_broadcast (m1_mode);
> +  if (need_mask_operand_p (insn_flags))
> +    emit_nonvlmax_insn (icode, SCALAR_MOVE_OP, scalar_move_ops, ops[3]);
> +  else
> +    emit_vlmax_insn (icode, SCALAR_MOVE_OP, scalar_move_ops);
> +
>    rtx m1_tmp2 = gen_reg_rtx (m1_mode);
>    rtx reduc_ops[] = {m1_tmp2, vector_src, m1_tmp};
> -  insn_code icode = code_for_pred (unspec, vmode);
> +  icode = code_for_pred (unspec, vmode);
>
>    if (need_mask_operand_p (insn_flags))
>      {
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113697.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113697.c
> new file mode 100644
> index 00000000000..588b86c7e6c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr113697.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -march=rv64gcv -mabi=lp64d -fno-schedule-insns" } */
> +
> +int
> +foo (int *__restrict a, int n)
> +{
> +  int result = 0;
> +  for (int i = 0; i < n; i++)
> +    result += a[i];
> +  return result;
> +}
> +
> +/* { dg-final { scan-assembler-times {vsetvli} 3 } } */
> +/* { dg-final { scan-assembler-not {vsetivli} } } */
> --
> 2.36.3
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] RISC-V: Expand VLMAX scalar move in reduction
  2024-02-02  1:56 [PATCH] RISC-V: Expand VLMAX scalar move in reduction Juzhe-Zhong
  2024-02-02  9:50 ` Kito Cheng
@ 2024-02-05  3:26 ` Jeff Law
  2024-02-05  4:36   ` Jeff Law
  1 sibling, 1 reply; 7+ messages in thread
From: Jeff Law @ 2024-02-05  3:26 UTC (permalink / raw)
  To: Juzhe-Zhong, gcc-patches; +Cc: kito.cheng, kito.cheng, rdapp.gcc



On 2/1/24 18:56, Juzhe-Zhong wrote:
> This patch fixes the following:
> 
>          vsetvli a5,a1,e32,m1,tu,ma
>          slli    a4,a5,2
>          sub     a1,a1,a5
>          vle32.v v2,0(a0)
>          add     a0,a0,a4
>          vadd.vv v1,v2,v1
>          bne     a1,zero,.L3
>          vsetivli        zero,1,e32,m1,ta,ma
>          vmv.s.x v2,zero
>          vsetvli a5,zero,e32,m1,ta,ma              ---> Redundant vsetvl.
>          vredsum.vs      v1,v1,v2
>          vmv.x.s a0,v1
>          ret
> 
> VSETVL PASS is able to fuse avl = 1 of scalar move and VLMAX avl of reduction.
> 
> However, this following RTL blocks the fusion in dependence analysis in VSETVL PASS:
> 
> (insn 49 24 50 5 (set (reg:RVVM1SI 98 v2 [148])
>          (if_then_else:RVVM1SI (unspec:RVVMF32BI [
>                      (const_vector:RVVMF32BI [
>                              (const_int 1 [0x1])
>                              repeat [
>                                  (const_int 0 [0])
>                              ]
>                          ])
>                      (const_int 1 [0x1])
>                      (const_int 2 [0x2]) repeated x2
>                      (const_int 0 [0])
>                      (reg:SI 66 vl)
>                      (reg:SI 67 vtype)
>                  ] UNSPEC_VPREDICATE)
>              (const_vector:RVVM1SI repeat [
>                      (const_int 0 [0])
>                  ])
>              (unspec:RVVM1SI [
>                      (reg:DI 0 zero)
>                  ] UNSPEC_VUNDEF))) 3813 {*pred_broadcastrvvm1si_zero}
>       (nil))
> (insn 50 49 51 5 (set (reg:DI 15 a5 [151])                          ---->  It set a5, blocks the following VLMAX into the scalar move above.
>          (unspec:DI [
>                  (const_int 32 [0x20])
>              ] UNSPEC_VLMAX)) 2566 {vlmax_avldi}
>       (expr_list:REG_EQUIV (unspec:DI [
>                  (const_int 32 [0x20])
>              ] UNSPEC_VLMAX)
>          (nil)))
> (insn 51 50 52 5 (set (reg:RVVM1SI 97 v1 [150])
>          (unspec:RVVM1SI [
>                  (unspec:RVVMF32BI [
>                          (const_vector:RVVMF32BI repeat [
>                                  (const_int 1 [0x1])
>                              ])
>                          (reg:DI 15 a5 [151])
>                          (const_int 2 [0x2])
>                          (const_int 1 [0x1])
>                          (reg:SI 66 vl)
>                          (reg:SI 67 vtype)
>                      ] UNSPEC_VPREDICATE)
>                  (unspec:RVVM1SI [
>                          (reg:RVVM1SI 97 v1 [orig:134 vect_result_14.6 ] [134])
>                          (reg:RVVM1SI 98 v2 [148])
>                      ] UNSPEC_REDUC_SUM)
>                  (unspec:RVVM1SI [
>                          (reg:DI 0 zero)
>                      ] UNSPEC_VUNDEF)
>              ] UNSPEC_REDUC)) 17541 {pred_redsumrvvm1si}
>       (expr_list:REG_DEAD (reg:RVVM1SI 98 v2 [148])
>          (expr_list:REG_DEAD (reg:SI 66 vl)
>              (expr_list:REG_DEAD (reg:DI 15 a5 [151])
>                  (expr_list:REG_DEAD (reg:DI 0 zero)
>                      (nil))))))
> 
> Such situation can only happen on auto-vectorization, never happen on intrinsic codes.
> Since the reduction is passed VLMAX AVL, it should be more natural to pass VLMAX to the scalar move which initial the value of the reduction.
> 
> After this patch:
> 
> 	vsetvli	a5,a1,e32,m1,tu,ma
> 	slli	a4,a5,2
> 	sub	a1,a1,a5
> 	vle32.v	v2,0(a0)
> 	add	a0,a0,a4
> 	vadd.vv	v1,v2,v1
> 	bne	a1,zero,.L3
> 	vsetvli	a5,zero,e32,m1,ta,ma
> 	vmv.s.x	v2,zero
> 	vredsum.vs	v1,v1,v2
> 	vmv.x.s	a0,v1
>          ret
> 
> Tested on both RV32/RV64 no regression.
> 
> 	PR target/113697
> 
> gcc/ChangeLog:
> 
> 	* config/riscv/riscv-v.cc (expand_reduction): Pass VLMAX avl to scalar move.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/riscv/rvv/autovec/pr113697.c: New test.
I suspect this broke 502.gcc in spec2017.  Basically it's hanging during 
the build phase.  I'm not sure if I'm going to have time this week to 
dive into it.


Optimization options used:

> GCC Flags:  -Ofast -flto -fsched-pressure -fno-strict-aliasing -fgnu89-inline -fcommon -fno-finite-math-only -fno-unsafe-math-optimizations



Given this appears to be a minor optimization issue, I wouldn't lose any 
sleep if it was reverted and deferred to gcc-15.

Anyway, good luck.  Sorry I can't do more on the debugging/reduction front.

Jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] RISC-V: Expand VLMAX scalar move in reduction
  2024-02-05  3:26 ` Jeff Law
@ 2024-02-05  4:36   ` Jeff Law
  2024-02-05  6:37     ` juzhe.zhong
  0 siblings, 1 reply; 7+ messages in thread
From: Jeff Law @ 2024-02-05  4:36 UTC (permalink / raw)
  To: Juzhe-Zhong, gcc-patches; +Cc: kito.cheng, kito.cheng, rdapp.gcc



On 2/4/24 20:26, Jeff Law wrote:
> 
> 
> On 2/1/24 18:56, Juzhe-Zhong wrote:
>> This patch fixes the following:
>>
>>          vsetvli a5,a1,e32,m1,tu,ma
>>          slli    a4,a5,2
>>          sub     a1,a1,a5
>>          vle32.v v2,0(a0)
>>          add     a0,a0,a4
>>          vadd.vv v1,v2,v1
>>          bne     a1,zero,.L3
>>          vsetivli        zero,1,e32,m1,ta,ma
>>          vmv.s.x v2,zero
>>          vsetvli a5,zero,e32,m1,ta,ma              ---> Redundant vsetvl.
>>          vredsum.vs      v1,v1,v2
>>          vmv.x.s a0,v1
>>          ret
>>
>> VSETVL PASS is able to fuse avl = 1 of scalar move and VLMAX avl of 
>> reduction.
>>
>> However, this following RTL blocks the fusion in dependence analysis 
>> in VSETVL PASS:
>>
>> (insn 49 24 50 5 (set (reg:RVVM1SI 98 v2 [148])
>>          (if_then_else:RVVM1SI (unspec:RVVMF32BI [
>>                      (const_vector:RVVMF32BI [
>>                              (const_int 1 [0x1])
>>                              repeat [
>>                                  (const_int 0 [0])
>>                              ]
>>                          ])
>>                      (const_int 1 [0x1])
>>                      (const_int 2 [0x2]) repeated x2
>>                      (const_int 0 [0])
>>                      (reg:SI 66 vl)
>>                      (reg:SI 67 vtype)
>>                  ] UNSPEC_VPREDICATE)
>>              (const_vector:RVVM1SI repeat [
>>                      (const_int 0 [0])
>>                  ])
>>              (unspec:RVVM1SI [
>>                      (reg:DI 0 zero)
>>                  ] UNSPEC_VUNDEF))) 3813 {*pred_broadcastrvvm1si_zero}
>>       (nil))
>> (insn 50 49 51 5 (set (reg:DI 15 a5 [151])                          
>> ---->  It set a5, blocks the following VLMAX into the scalar move above.
>>          (unspec:DI [
>>                  (const_int 32 [0x20])
>>              ] UNSPEC_VLMAX)) 2566 {vlmax_avldi}
>>       (expr_list:REG_EQUIV (unspec:DI [
>>                  (const_int 32 [0x20])
>>              ] UNSPEC_VLMAX)
>>          (nil)))
>> (insn 51 50 52 5 (set (reg:RVVM1SI 97 v1 [150])
>>          (unspec:RVVM1SI [
>>                  (unspec:RVVMF32BI [
>>                          (const_vector:RVVMF32BI repeat [
>>                                  (const_int 1 [0x1])
>>                              ])
>>                          (reg:DI 15 a5 [151])
>>                          (const_int 2 [0x2])
>>                          (const_int 1 [0x1])
>>                          (reg:SI 66 vl)
>>                          (reg:SI 67 vtype)
>>                      ] UNSPEC_VPREDICATE)
>>                  (unspec:RVVM1SI [
>>                          (reg:RVVM1SI 97 v1 [orig:134 vect_result_14.6 
>> ] [134])
>>                          (reg:RVVM1SI 98 v2 [148])
>>                      ] UNSPEC_REDUC_SUM)
>>                  (unspec:RVVM1SI [
>>                          (reg:DI 0 zero)
>>                      ] UNSPEC_VUNDEF)
>>              ] UNSPEC_REDUC)) 17541 {pred_redsumrvvm1si}
>>       (expr_list:REG_DEAD (reg:RVVM1SI 98 v2 [148])
>>          (expr_list:REG_DEAD (reg:SI 66 vl)
>>              (expr_list:REG_DEAD (reg:DI 15 a5 [151])
>>                  (expr_list:REG_DEAD (reg:DI 0 zero)
>>                      (nil))))))
>>
>> Such situation can only happen on auto-vectorization, never happen on 
>> intrinsic codes.
>> Since the reduction is passed VLMAX AVL, it should be more natural to 
>> pass VLMAX to the scalar move which initial the value of the reduction.
>>
>> After this patch:
>>
>>     vsetvli    a5,a1,e32,m1,tu,ma
>>     slli    a4,a5,2
>>     sub    a1,a1,a5
>>     vle32.v    v2,0(a0)
>>     add    a0,a0,a4
>>     vadd.vv    v1,v2,v1
>>     bne    a1,zero,.L3
>>     vsetvli    a5,zero,e32,m1,ta,ma
>>     vmv.s.x    v2,zero
>>     vredsum.vs    v1,v1,v2
>>     vmv.x.s    a0,v1
>>          ret
>>
>> Tested on both RV32/RV64 no regression.
>>
>>     PR target/113697
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv/riscv-v.cc (expand_reduction): Pass VLMAX avl to 
>> scalar move.
>>
>> gcc/testsuite/ChangeLog:
>>
>>     * gcc.target/riscv/rvv/autovec/pr113697.c: New test.
> I suspect this broke 502.gcc in spec2017.  Basically it's hanging during 
> the build phase.  I'm not sure if I'm going to have time this week to 
> dive into it.
> 
> 
> Optimization options used:
> 
>> GCC Flags:  -Ofast -flto -fsched-pressure -fno-strict-aliasing 
>> -fgnu89-inline -fcommon -fno-finite-math-only 
>> -fno-unsafe-math-optimizations
> 
> 
> 
> Given this appears to be a minor optimization issue, I wouldn't lose any 
> sleep if it was reverted and deferred to gcc-15.
> 
> Anyway, good luck.  Sorry I can't do more on the debugging/reduction front.
Actually, I'm starting to wonder if this is just the trigger and if the 
real issue is something else that went in over the last week or so.  I 
reverted the patch above which allows 502.gcc to build. But then I get a 
hang on xalancbmk.

Makes me wonder if the vsetvl bits are the culprit given the size of 
that change.

jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Re: [PATCH] RISC-V: Expand VLMAX scalar move in reduction
  2024-02-05  4:36   ` Jeff Law
@ 2024-02-05  6:37     ` juzhe.zhong
  2024-02-05 15:34       ` Jeff Law
  2024-02-05 19:09       ` Jeff Law
  0 siblings, 2 replies; 7+ messages in thread
From: juzhe.zhong @ 2024-02-05  6:37 UTC (permalink / raw)
  To: jeffreyalaw, gcc-patches; +Cc: kito.cheng, Kito.cheng, Robin Dapp

[-- Attachment #1: Type: text/plain, Size: 5596 bytes --]

I think it just trigger a latent bug that we didn't encounter.

Hi, Robin. Would you mind give me preprocessed file to reproduce the issue ?

I suspect it triggers latent bug in VSETVL PASS.



juzhe.zhong@rivai.ai
 
From: Jeff Law
Date: 2024-02-05 12:36
To: Juzhe-Zhong; gcc-patches
CC: kito.cheng; kito.cheng; rdapp.gcc
Subject: Re: [PATCH] RISC-V: Expand VLMAX scalar move in reduction
 
 
On 2/4/24 20:26, Jeff Law wrote:
> 
> 
> On 2/1/24 18:56, Juzhe-Zhong wrote:
>> This patch fixes the following:
>>
>>          vsetvli a5,a1,e32,m1,tu,ma
>>          slli    a4,a5,2
>>          sub     a1,a1,a5
>>          vle32.v v2,0(a0)
>>          add     a0,a0,a4
>>          vadd.vv v1,v2,v1
>>          bne     a1,zero,.L3
>>          vsetivli        zero,1,e32,m1,ta,ma
>>          vmv.s.x v2,zero
>>          vsetvli a5,zero,e32,m1,ta,ma              ---> Redundant vsetvl.
>>          vredsum.vs      v1,v1,v2
>>          vmv.x.s a0,v1
>>          ret
>>
>> VSETVL PASS is able to fuse avl = 1 of scalar move and VLMAX avl of 
>> reduction.
>>
>> However, this following RTL blocks the fusion in dependence analysis 
>> in VSETVL PASS:
>>
>> (insn 49 24 50 5 (set (reg:RVVM1SI 98 v2 [148])
>>          (if_then_else:RVVM1SI (unspec:RVVMF32BI [
>>                      (const_vector:RVVMF32BI [
>>                              (const_int 1 [0x1])
>>                              repeat [
>>                                  (const_int 0 [0])
>>                              ]
>>                          ])
>>                      (const_int 1 [0x1])
>>                      (const_int 2 [0x2]) repeated x2
>>                      (const_int 0 [0])
>>                      (reg:SI 66 vl)
>>                      (reg:SI 67 vtype)
>>                  ] UNSPEC_VPREDICATE)
>>              (const_vector:RVVM1SI repeat [
>>                      (const_int 0 [0])
>>                  ])
>>              (unspec:RVVM1SI [
>>                      (reg:DI 0 zero)
>>                  ] UNSPEC_VUNDEF))) 3813 {*pred_broadcastrvvm1si_zero}
>>       (nil))
>> (insn 50 49 51 5 (set (reg:DI 15 a5 [151])                          
>> ---->  It set a5, blocks the following VLMAX into the scalar move above.
>>          (unspec:DI [
>>                  (const_int 32 [0x20])
>>              ] UNSPEC_VLMAX)) 2566 {vlmax_avldi}
>>       (expr_list:REG_EQUIV (unspec:DI [
>>                  (const_int 32 [0x20])
>>              ] UNSPEC_VLMAX)
>>          (nil)))
>> (insn 51 50 52 5 (set (reg:RVVM1SI 97 v1 [150])
>>          (unspec:RVVM1SI [
>>                  (unspec:RVVMF32BI [
>>                          (const_vector:RVVMF32BI repeat [
>>                                  (const_int 1 [0x1])
>>                              ])
>>                          (reg:DI 15 a5 [151])
>>                          (const_int 2 [0x2])
>>                          (const_int 1 [0x1])
>>                          (reg:SI 66 vl)
>>                          (reg:SI 67 vtype)
>>                      ] UNSPEC_VPREDICATE)
>>                  (unspec:RVVM1SI [
>>                          (reg:RVVM1SI 97 v1 [orig:134 vect_result_14.6 
>> ] [134])
>>                          (reg:RVVM1SI 98 v2 [148])
>>                      ] UNSPEC_REDUC_SUM)
>>                  (unspec:RVVM1SI [
>>                          (reg:DI 0 zero)
>>                      ] UNSPEC_VUNDEF)
>>              ] UNSPEC_REDUC)) 17541 {pred_redsumrvvm1si}
>>       (expr_list:REG_DEAD (reg:RVVM1SI 98 v2 [148])
>>          (expr_list:REG_DEAD (reg:SI 66 vl)
>>              (expr_list:REG_DEAD (reg:DI 15 a5 [151])
>>                  (expr_list:REG_DEAD (reg:DI 0 zero)
>>                      (nil))))))
>>
>> Such situation can only happen on auto-vectorization, never happen on 
>> intrinsic codes.
>> Since the reduction is passed VLMAX AVL, it should be more natural to 
>> pass VLMAX to the scalar move which initial the value of the reduction.
>>
>> After this patch:
>>
>>     vsetvli    a5,a1,e32,m1,tu,ma
>>     slli    a4,a5,2
>>     sub    a1,a1,a5
>>     vle32.v    v2,0(a0)
>>     add    a0,a0,a4
>>     vadd.vv    v1,v2,v1
>>     bne    a1,zero,.L3
>>     vsetvli    a5,zero,e32,m1,ta,ma
>>     vmv.s.x    v2,zero
>>     vredsum.vs    v1,v1,v2
>>     vmv.x.s    a0,v1
>>          ret
>>
>> Tested on both RV32/RV64 no regression.
>>
>>     PR target/113697
>>
>> gcc/ChangeLog:
>>
>>     * config/riscv/riscv-v.cc (expand_reduction): Pass VLMAX avl to 
>> scalar move.
>>
>> gcc/testsuite/ChangeLog:
>>
>>     * gcc.target/riscv/rvv/autovec/pr113697.c: New test.
> I suspect this broke 502.gcc in spec2017.  Basically it's hanging during 
> the build phase.  I'm not sure if I'm going to have time this week to 
> dive into it.
> 
> 
> Optimization options used:
> 
>> GCC Flags:  -Ofast -flto -fsched-pressure -fno-strict-aliasing 
>> -fgnu89-inline -fcommon -fno-finite-math-only 
>> -fno-unsafe-math-optimizations
> 
> 
> 
> Given this appears to be a minor optimization issue, I wouldn't lose any 
> sleep if it was reverted and deferred to gcc-15.
> 
> Anyway, good luck.  Sorry I can't do more on the debugging/reduction front.
Actually, I'm starting to wonder if this is just the trigger and if the 
real issue is something else that went in over the last week or so.  I 
reverted the patch above which allows 502.gcc to build. But then I get a 
hang on xalancbmk.
 
Makes me wonder if the vsetvl bits are the culprit given the size of 
that change.
 
jeff
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] RISC-V: Expand VLMAX scalar move in reduction
  2024-02-05  6:37     ` juzhe.zhong
@ 2024-02-05 15:34       ` Jeff Law
  2024-02-05 19:09       ` Jeff Law
  1 sibling, 0 replies; 7+ messages in thread
From: Jeff Law @ 2024-02-05 15:34 UTC (permalink / raw)
  To: juzhe.zhong, gcc-patches; +Cc: kito.cheng, Kito.cheng, Robin Dapp



On 2/4/24 23:37, juzhe.zhong@rivai.ai wrote:
> I think it just trigger a latent bug that we didn't encounter.
> 
> Hi, Robin. Would you mind give me preprocessed file to reproduce the issue ?
> 
> I suspect it triggers latent bug in VSETVL PASS.
I've got a few minutes this morning before meetings start.  I'm going to 
try and trigger this with LTO off, which would help dramatically with 
our ability to provide a testcase.

jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] RISC-V: Expand VLMAX scalar move in reduction
  2024-02-05  6:37     ` juzhe.zhong
  2024-02-05 15:34       ` Jeff Law
@ 2024-02-05 19:09       ` Jeff Law
  1 sibling, 0 replies; 7+ messages in thread
From: Jeff Law @ 2024-02-05 19:09 UTC (permalink / raw)
  To: juzhe.zhong, gcc-patches; +Cc: kito.cheng, Kito.cheng, Robin Dapp



On 2/4/24 23:37, juzhe.zhong@rivai.ai wrote:
> I think it just trigger a latent bug that we didn't encounter.
> 
> Hi, Robin. Would you mind give me preprocessed file to reproduce the issue ?
> 
> I suspect it triggers latent bug in VSETVL PASS.
So it looks like vsetvl has made a transformation that makes DCE go into 
a loop.  At least that's my first impression after attaching to a hung 
build.  The good news is I was able to trigger it without LTO.  I'll 
send the relevant info separately so as not to spam everyone with the 
testcase :-)

jeff

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-02-05 19:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-02  1:56 [PATCH] RISC-V: Expand VLMAX scalar move in reduction Juzhe-Zhong
2024-02-02  9:50 ` Kito Cheng
2024-02-05  3:26 ` Jeff Law
2024-02-05  4:36   ` Jeff Law
2024-02-05  6:37     ` juzhe.zhong
2024-02-05 15:34       ` Jeff Law
2024-02-05 19:09       ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).