[PATCH V2] RISC-V: Fix incorrect VXRM configuration in mode switching for CALL and ASM

[Kito Cheng]

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
>