Re: [PATCH] RISC-V: Allow Vector IOR(V1, NOT V1) optimiztion

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Apr 18, 2023 at 3:31 AM Li, Pan2 via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Passed the X86 bootstrap and regression tests.
>
> Pan
>
> -----Original Message-----
> From: Li, Pan2 <pan2.li@intel.com>
> Sent: Monday, April 17, 2023 10:50 PM
> To: gcc-patches@gcc.gnu.org
> Cc: juzhe.zhong@rivai.ai; kito.cheng@sifive.com; rguenther@suse.de; Li, Pan2 <pan2.li@intel.com>; Wang, Yanzhang <yanzhang.wang@intel.com>; richard.sandiford@arm.com
> Subject: [PATCH] RISC-V: Allow Vector IOR(V1, NOT V1) optimiztion
>
> From: Pan Li <pan2.li@intel.com>
>
> This patch add the optimization for the vector IOR(V1, NOT V1). Assume we have below sample code.
>
> vbool32_t test_shortcut_for_riscv_vmorn_case_5(vbool32_t v1, size_t vl) {
>   return __riscv_vmorn_mm_b32(v1, v1, vl); }
>
> Before this patch:
> vsetvli  a5,zero,e8,mf4,ta,ma
> vlm.v    v24,0(a1)
> vsetvli  zero,a2,e8,mf4,ta,ma
> vmorn.mm v24,v24,v24
> vsetvli  a5,zero,e8,mf4,ta,ma
> vsm.v    v24,0(a0)
> ret
>
> After this patch:
> vsetvli zero,a2,e8,mf4,ta,ma
> vmset.m v24
> vsetvli a5,zero,e8,mf4,ta,ma
> vsm.v   v24,0(a0)
> ret
>
> Or in RTL's perspective,
> from:
> (ior:VNx2BI (reg/v:VNx2BI 137 [ v1 ]) (not:VNx2BI (reg/v:VNx2BI 137 [ v1 ])))
> to:
> (const_vector:VNx2BI repeat [ (const_int 1 [0x1]) ])
>
> The similar optimization like VMANDN has enabled already. There should be no difference execpt the operator when compare the VMORN and VMANDN for such kind of optimization. The patch allows the VECTOR_BOOL IOR(V1, NOT V1) simplification besides the existing SCALAR_INT mode.
>
> gcc/ChangeLog:
>
>         * machmode.h (VECTOR_BOOL_MODE_P):
>         * simplify-rtx.cc (valid_mode_for_ior_simplification_p):
>         (simplify_context::simplify_binary_operation_1):
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/base/mask_insn_shortcut.c:
>         * gcc.target/riscv/simplify_ior_optimization.c: New test.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/machmode.h                                |  4 ++
>  gcc/simplify-rtx.cc                           | 10 +++-
>  .../riscv/rvv/base/mask_insn_shortcut.c       |  3 +-
>  .../riscv/simplify_ior_optimization.c         | 50 +++++++++++++++++++
>  4 files changed, 63 insertions(+), 4 deletions(-)  create mode 100644 gcc/testsuite/gcc.target/riscv/simplify_ior_optimization.c
>
> diff --git a/gcc/machmode.h b/gcc/machmode.h index f1865c1ef42..771bae89cb7 100644
> --- a/gcc/machmode.h
> +++ b/gcc/machmode.h
> @@ -134,6 +134,10 @@ extern const unsigned char mode_class[NUM_MACHINE_MODES];
>     || GET_MODE_CLASS (MODE) == MODE_VECTOR_ACCUM       \
>     || GET_MODE_CLASS (MODE) == MODE_VECTOR_UACCUM)
>
> +/* Nonzero if MODE is a vector bool mode.  */
> +#define VECTOR_BOOL_MODE_P(MODE)                       \
> +  (GET_MODE_CLASS (MODE) == MODE_VECTOR_BOOL)          \
> +
>  /* Nonzero if MODE is a scalar integral mode.  */
>  #define SCALAR_INT_MODE_P(MODE)                        \
>    (GET_MODE_CLASS (MODE) == MODE_INT           \
> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc index ee75079917f..eff27b835bf 100644
> --- a/gcc/simplify-rtx.cc
> +++ b/gcc/simplify-rtx.cc
> @@ -57,6 +57,12 @@ neg_poly_int_rtx (machine_mode mode, const_rtx i)
>    return immed_wide_int_const (-wi::to_poly_wide (i, mode), mode);  }
>
> +static bool
> +valid_mode_for_ior_simplification_p (machine_mode mode) {
> +  return SCALAR_INT_MODE_P (mode) || VECTOR_BOOL_MODE_P (mode); }
> +
>  /* Test whether expression, X, is an immediate constant that represents
>     the most significant bit of machine mode MODE.  */
>
> @@ -3332,8 +3338,8 @@ simplify_context::simplify_binary_operation_1 (rtx_code code,
>        if (((GET_CODE (op0) == NOT && rtx_equal_p (XEXP (op0, 0), op1))
>            || (GET_CODE (op1) == NOT && rtx_equal_p (XEXP (op1, 0), op0)))
>           && ! side_effects_p (op0)
> -         && SCALAR_INT_MODE_P (mode))
> -       return constm1_rtx;
> +         && valid_mode_for_ior_simplification_p (mode))

for simple predicates like this please do not split them out, it makes
understanding the code more difficult.

> +       return CONST1_RTX (mode);

shouldn't this be CONSTM1_RTX (mode)?  Why is this only valid for VECTOR_BOOL
and not also for VECTOR_INT?  You're citing AND and that does

      /* A & (~A) -> 0 */
      if (((GET_CODE (op0) == NOT && rtx_equal_p (XEXP (op0, 0), op1))
           || (GET_CODE (op1) == NOT && rtx_equal_p (XEXP (op1, 0), op0)))
          && ! side_effects_p (op0)
          && GET_MODE_CLASS (mode) != MODE_CC)
        return CONST0_RTX (mode);

so why differ and not use the same GET_MODE_CLASS (mode) != MODE_CC condition?

Richard.

>
>        /* (ior A C) is C if all bits of A that might be nonzero are on in C.  */
>        if (CONST_INT_P (op1)
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/mask_insn_shortcut.c b/gcc/testsuite/gcc.target/riscv/rvv/base/mask_insn_shortcut.c
> index 83cc4a1b5a5..57d0241675a 100644
> --- a/gcc/testsuite/gcc.target/riscv/rvv/base/mask_insn_shortcut.c
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/mask_insn_shortcut.c
> @@ -233,9 +233,8 @@ vbool64_t test_shortcut_for_riscv_vmxnor_case_6(vbool64_t v1, size_t vl) {
>  /* { dg-final { scan-assembler-not {vmxor\.mm\s+v[0-9]+,\s*v[0-9]+} } } */
>  /* { dg-final { scan-assembler-not {vmor\.mm\s+v[0-9]+,\s*v[0-9]+} } } */
>  /* { dg-final { scan-assembler-not {vmnor\.mm\s+v[0-9]+,\s*v[0-9]+} } } */
> -/* { dg-final { scan-assembler-times {vmorn\.mm\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+} 7 } } */
>  /* { dg-final { scan-assembler-not {vmxnor\.mm\s+v[0-9]+,\s*v[0-9]+} } } */
>  /* { dg-final { scan-assembler-times {vmclr\.m\s+v[0-9]+} 14 } } */
> -/* { dg-final { scan-assembler-times {vmset\.m\s+v[0-9]+} 7 } } */
> +/* { dg-final { scan-assembler-times {vmset\.m\s+v[0-9]+} 14 } } */
>  /* { dg-final { scan-assembler-times {vmmv\.m\s+v[0-9]+,\s*v[0-9]+} 14 } } */
>  /* { dg-final { scan-assembler-times {vmnot\.m\s+v[0-9]+,\s*v[0-9]+} 14 } } */ diff --git a/gcc/testsuite/gcc.target/riscv/simplify_ior_optimization.c b/gcc/testsuite/gcc.target/riscv/simplify_ior_optimization.c
> new file mode 100644
> index 00000000000..ec3bd0baf03
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/simplify_ior_optimization.c
> @@ -0,0 +1,50 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64 -O2" } */
> +
> +#include <stdint.h>
> +
> +uint8_t test_simplify_ior_scalar_case_0 (uint8_t a) {
> +  return a | ~a;
> +}
> +
> +uint16_t test_simplify_ior_scalar_case_1 (uint16_t a) {
> +  return a | ~a;
> +}
> +
> +uint32_t test_simplify_ior_scalar_case_2 (uint32_t a) {
> +  return a | ~a;
> +}
> +
> +uint64_t test_simplify_ior_scalar_case_3 (uint64_t a) {
> +  return a | ~a;
> +}
> +
> +int8_t test_simplify_ior_scalar_case_4 (int8_t a) {
> +  return a | ~a;
> +}
> +
> +int16_t test_simplify_ior_scalar_case_5 (int16_t a) {
> +  return a | ~a;
> +}
> +
> +int32_t test_simplify_ior_scalar_case_6 (int32_t a) {
> +  return a | ~a;
> +}
> +
> +int64_t test_simplify_ior_scalar_case_7 (int64_t a) {
> +  return a | ~a;
> +}
> +
> +/* { dg-final { scan-assembler-times {li\s+a[0-9]+,\s*-1} 6 } } */
> +/* { dg-final { scan-assembler-times {li\s+a[0-9]+,\s*255} 1 } } */
> +/* { dg-final { scan-assembler-times {li\s+a[0-9]+,\s*65536} 1 } } */
> +/* { dg-final { scan-assembler-not {or\s+a[0-9]+} } } */
> +/* { dg-final { scan-assembler-not {not\s+a[0-9]+} } } */
> --
> 2.34.1
>