public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <jeffreyalaw@gmail.com>
To: Juzhe-Zhong <juzhe.zhong@rivai.ai>, gcc-patches@gcc.gnu.org
Cc: kito.cheng@gmail.com, kito.cheng@sifive.com, rdapp.gcc@gmail.com
Subject: Re: [PATCH] RISC-V: Expand VLMAX scalar move in reduction
Date: Sun, 4 Feb 2024 20:26:52 -0700	[thread overview]
Message-ID: <8d5529d6-df50-4ac2-9bdb-a7525142b956@gmail.com> (raw)
In-Reply-To: <20240202015659.54072-1-juzhe.zhong@rivai.ai>



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

  parent reply	other threads:[~2024-02-05  3:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-02  1:56 Juzhe-Zhong
2024-02-02  9:50 ` Kito Cheng
2024-02-05  3:26 ` Jeff Law [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8d5529d6-df50-4ac2-9bdb-a7525142b956@gmail.com \
    --to=jeffreyalaw@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=juzhe.zhong@rivai.ai \
    --cc=kito.cheng@gmail.com \
    --cc=kito.cheng@sifive.com \
    --cc=rdapp.gcc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).