| Message ID | 20230525065957.1872100-1-juzhe.zhong@rivai.ai |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] RISC-V: Fix incorrect VXRM configuration in mode switching for CALL and ASM | expand |
LGTM, thanks :) On Thu, May 25, 2023 at 3:00 PM <juzhe.zhong@rivai.ai> wrote: > > From: Juzhe-Zhong <juzhe.zhong@rivai.ai> > > Currently mode switching incorrect codegen for the following case: > void fn (void); > > void f (void * in, void *out, int32_t x, int n, int m) > { > for (int i = 0; i < n; i++) { > vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); > vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); > vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); > fn (); > v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); > __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); > } > } > > Before this patch: > > Preheader: > ... > csrwi vxrm,2 > Loop Body: > ... (no cswri vxrm,2) > vaadd.vx > ... > vaadd.vx > ... > > This codegen is incorrect. > > After this patch: > > Preheader: > ... > csrwi vxrm,2 > Loop Body: > ... > vaadd.vx > ... > csrwi vxrm,2 > ... > vaadd.vx > ... > > cross-compile build PASS and regression PASS > > Ok for trunk ? > > gcc/ChangeLog: > > * config/riscv/riscv.cc (global_state_unknown_p): New function. > (riscv_mode_after): Fix incorrect VXM. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/rvv/base/vxrm-11.c: New test. > * gcc.target/riscv/rvv/base/vxrm-12.c: New test. > > --- > gcc/config/riscv/riscv.cc | 29 ++++++++++++++++++- > .../gcc.target/riscv/rvv/base/vxrm-11.c | 20 +++++++++++++ > .../gcc.target/riscv/rvv/base/vxrm-12.c | 18 ++++++++++++ > 3 files changed, 66 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 09fc9e5d95e..406c5469425 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -7549,6 +7549,31 @@ riscv_mode_needed (int entity, rtx_insn *insn) > } > } > > +/* Return true if the VXRM/FRM status of the INSN is unknown. */ > +static bool > +global_state_unknown_p (rtx_insn *insn, unsigned int regno) > +{ > + struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn); > + df_ref ref; > + > + /* Return true if there is a definition of VXRM. */ > + for (ref = DF_INSN_INFO_DEFS (insn_info); ref; ref = DF_REF_NEXT_LOC (ref)) > + if (DF_REF_REGNO (ref) == regno) > + return true; > + > + /* A CALL function may contain an instruction that modifies the VXRM, > + return true in this situation. */ > + if (CALL_P (insn)) > + return true; > + > + /* Return true for all assembly since users may hardcode a assembly > + like this: asm volatile ("csrwi vxrm, 0"). */ > + extract_insn (insn); > + if (recog_data.is_asm) > + return true; > + return false; > +} > + > /* Return the mode that an insn results in. */ > > static int > @@ -7557,7 +7582,9 @@ riscv_mode_after (int entity, int mode, rtx_insn *insn) > switch (entity) > { > case RISCV_VXRM: > - if (recog_memoized (insn) >= 0) > + if (global_state_unknown_p (insn, VXRM_REGNUM)) > + return VXRM_MODE_NONE; > + else if (recog_memoized (insn) >= 0) > return reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), > PATTERN (insn)) > ? get_attr_vxrm_mode (insn) > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c > new file mode 100644 > index 00000000000..7f637a8b7f5 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c > @@ -0,0 +1,20 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ > + > +#include "riscv_vector.h" > + > +void fn (void); > + > +void f (void * in, void *out, int32_t x, int n, int m) > +{ > + for (int i = 0; i < n; i++) { > + vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); > + vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); > + vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); > + fn (); > + v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); > + __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); > + } > +} > + > +/* { dg-final { scan-assembler-times {csrwi\s+vxrm,\s*2} 2 } } */ > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c > new file mode 100644 > index 00000000000..c3ab509f106 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c > @@ -0,0 +1,18 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ > + > +#include "riscv_vector.h" > + > +void f (void * in, void *out, int32_t x, int n, int m) > +{ > + for (int i = 0; i < n; i++) { > + vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); > + vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); > + vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); > + asm volatile ("csrwi\tvxrm,1"); > + v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); > + __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); > + } > +} > + > +/* { dg-final { scan-assembler-times {csrwi\s+vxrm,\s*2} 2 } } */ > -- > 2.36.3 >
Committed, thanks Kito. Pan -----Original Message----- From: Kito Cheng <kito.cheng@gmail.com> Sent: Monday, May 29, 2023 10:22 AM To: juzhe.zhong@rivai.ai Cc: gcc-patches@gcc.gnu.org; kito.cheng@sifive.com; palmer@dabbelt.com; palmer@rivosinc.com; jeffreyalaw@gmail.com; rdapp.gcc@gmail.com; Li, Pan2 <pan2.li@intel.com> Subject: Re: [PATCH V2] RISC-V: Fix incorrect VXRM configuration in mode switching for CALL and ASM LGTM, thanks :) On Thu, May 25, 2023 at 3:00 PM <juzhe.zhong@rivai.ai> wrote: > > From: Juzhe-Zhong <juzhe.zhong@rivai.ai> > > Currently mode switching incorrect codegen for the following case: > void fn (void); > > void f (void * in, void *out, int32_t x, int n, int m) { > for (int i = 0; i < n; i++) { > vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); > vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); > vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); > fn (); > v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); > __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); > } > } > > Before this patch: > > Preheader: > ... > csrwi vxrm,2 > Loop Body: > ... (no cswri vxrm,2) > vaadd.vx > ... > vaadd.vx > ... > > This codegen is incorrect. > > After this patch: > > Preheader: > ... > csrwi vxrm,2 > Loop Body: > ... > vaadd.vx > ... > csrwi vxrm,2 > ... > vaadd.vx > ... > > cross-compile build PASS and regression PASS > > Ok for trunk ? > > gcc/ChangeLog: > > * config/riscv/riscv.cc (global_state_unknown_p): New function. > (riscv_mode_after): Fix incorrect VXM. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/rvv/base/vxrm-11.c: New test. > * gcc.target/riscv/rvv/base/vxrm-12.c: New test. > > --- > gcc/config/riscv/riscv.cc | 29 ++++++++++++++++++- > .../gcc.target/riscv/rvv/base/vxrm-11.c | 20 +++++++++++++ > .../gcc.target/riscv/rvv/base/vxrm-12.c | 18 ++++++++++++ > 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 > gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 09fc9e5d95e..406c5469425 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -7549,6 +7549,31 @@ riscv_mode_needed (int entity, rtx_insn *insn) > } > } > > +/* Return true if the VXRM/FRM status of the INSN is unknown. */ > +static bool global_state_unknown_p (rtx_insn *insn, unsigned int > +regno) { > + struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn); > + df_ref ref; > + > + /* Return true if there is a definition of VXRM. */ for (ref = > + DF_INSN_INFO_DEFS (insn_info); ref; ref = DF_REF_NEXT_LOC (ref)) > + if (DF_REF_REGNO (ref) == regno) > + return true; > + > + /* A CALL function may contain an instruction that modifies the VXRM, > + return true in this situation. */ if (CALL_P (insn)) > + return true; > + > + /* Return true for all assembly since users may hardcode a assembly > + like this: asm volatile ("csrwi vxrm, 0"). */ > + extract_insn (insn); > + if (recog_data.is_asm) > + return true; > + return false; > +} > + > /* Return the mode that an insn results in. */ > > static int > @@ -7557,7 +7582,9 @@ riscv_mode_after (int entity, int mode, rtx_insn *insn) > switch (entity) > { > case RISCV_VXRM: > - if (recog_memoized (insn) >= 0) > + if (global_state_unknown_p (insn, VXRM_REGNUM)) > + return VXRM_MODE_NONE; > + else if (recog_memoized (insn) >= 0) > return reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), > PATTERN (insn)) > ? get_attr_vxrm_mode (insn) diff --git > a/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c > b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c > new file mode 100644 > index 00000000000..7f637a8b7f5 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c > @@ -0,0 +1,20 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ > + > +#include "riscv_vector.h" > + > +void fn (void); > + > +void f (void * in, void *out, int32_t x, int n, int m) { > + for (int i = 0; i < n; i++) { > + vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); > + vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); > + vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); > + fn (); > + v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); > + __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); > + } > +} > + > +/* { dg-final { scan-assembler-times {csrwi\s+vxrm,\s*2} 2 } } */ > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c > b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c > new file mode 100644 > index 00000000000..c3ab509f106 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c > @@ -0,0 +1,18 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ > + > +#include "riscv_vector.h" > + > +void f (void * in, void *out, int32_t x, int n, int m) { > + for (int i = 0; i < n; i++) { > + vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); > + vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); > + vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); > + asm volatile ("csrwi\tvxrm,1"); > + v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); > + __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); > + } > +} > + > +/* { dg-final { scan-assembler-times {csrwi\s+vxrm,\s*2} 2 } } */ > -- > 2.36.3 >
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc index 09fc9e5d95e..406c5469425 100644 --- a/gcc/config/riscv/riscv.cc +++ b/gcc/config/riscv/riscv.cc @@ -7549,6 +7549,31 @@ riscv_mode_needed (int entity, rtx_insn *insn) } } +/* Return true if the VXRM/FRM status of the INSN is unknown. */ +static bool +global_state_unknown_p (rtx_insn *insn, unsigned int regno) +{ + struct df_insn_info *insn_info = DF_INSN_INFO_GET (insn); + df_ref ref; + + /* Return true if there is a definition of VXRM. */ + for (ref = DF_INSN_INFO_DEFS (insn_info); ref; ref = DF_REF_NEXT_LOC (ref)) + if (DF_REF_REGNO (ref) == regno) + return true; + + /* A CALL function may contain an instruction that modifies the VXRM, + return true in this situation. */ + if (CALL_P (insn)) + return true; + + /* Return true for all assembly since users may hardcode a assembly + like this: asm volatile ("csrwi vxrm, 0"). */ + extract_insn (insn); + if (recog_data.is_asm) + return true; + return false; +} + /* Return the mode that an insn results in. */ static int @@ -7557,7 +7582,9 @@ riscv_mode_after (int entity, int mode, rtx_insn *insn) switch (entity) { case RISCV_VXRM: - if (recog_memoized (insn) >= 0) + if (global_state_unknown_p (insn, VXRM_REGNUM)) + return VXRM_MODE_NONE; + else if (recog_memoized (insn) >= 0) return reg_mentioned_p (gen_rtx_REG (SImode, VXRM_REGNUM), PATTERN (insn)) ? get_attr_vxrm_mode (insn) diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c new file mode 100644 index 00000000000..7f637a8b7f5 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ + +#include "riscv_vector.h" + +void fn (void); + +void f (void * in, void *out, int32_t x, int n, int m) +{ + for (int i = 0; i < n; i++) { + vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); + vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); + vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); + fn (); + v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); + __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); + } +} + +/* { dg-final { scan-assembler-times {csrwi\s+vxrm,\s*2} 2 } } */ diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c new file mode 100644 index 00000000000..c3ab509f106 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */ + +#include "riscv_vector.h" + +void f (void * in, void *out, int32_t x, int n, int m) +{ + for (int i = 0; i < n; i++) { + vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); + vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); + vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); + asm volatile ("csrwi\tvxrm,1"); + v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); + __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); + } +} + +/* { dg-final { scan-assembler-times {csrwi\s+vxrm,\s*2} 2 } } */
From: Juzhe-Zhong <juzhe.zhong@rivai.ai> Currently mode switching incorrect codegen for the following case: void fn (void); void f (void * in, void *out, int32_t x, int n, int m) { for (int i = 0; i < n; i++) { vint32m1_t v = __riscv_vle32_v_i32m1 (in + i, 4); vint32m1_t v2 = __riscv_vle32_v_i32m1_tu (v, in + 100 + i, 4); vint32m1_t v3 = __riscv_vaadd_vx_i32m1 (v2, 0, VXRM_RDN, 4); fn (); v3 = __riscv_vaadd_vx_i32m1 (v3, 3, VXRM_RDN, 4); __riscv_vse32_v_i32m1 (out + 100 + i, v3, 4); } } Before this patch: Preheader: ... csrwi vxrm,2 Loop Body: ... (no cswri vxrm,2) vaadd.vx ... vaadd.vx ... This codegen is incorrect. After this patch: Preheader: ... csrwi vxrm,2 Loop Body: ... vaadd.vx ... csrwi vxrm,2 ... vaadd.vx ... cross-compile build PASS and regression PASS Ok for trunk ? gcc/ChangeLog: * config/riscv/riscv.cc (global_state_unknown_p): New function. (riscv_mode_after): Fix incorrect VXM. gcc/testsuite/ChangeLog: * gcc.target/riscv/rvv/base/vxrm-11.c: New test. * gcc.target/riscv/rvv/base/vxrm-12.c: New test. --- gcc/config/riscv/riscv.cc | 29 ++++++++++++++++++- .../gcc.target/riscv/rvv/base/vxrm-11.c | 20 +++++++++++++ .../gcc.target/riscv/rvv/base/vxrm-12.c | 18 ++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-11.c create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vxrm-12.c