RISC-V: Remove redundant vcond patterns
Checks
Commit Message
Previously, Richi has suggested that vcond patterns are only needed when target
support comparison + select consuming 1 instruction.
Now, I do the experiments on removing those "vcond" patterns, it works perfectly.
All testcases PASS.
Really appreicate Richi helps us recognize such issue.
Now remove all "vcond" patterns as Richi suggested.
gcc/ChangeLog:
* config/riscv/autovec.md (vcond<V:mode><VI:mode>): Remove redundant vcond patterns.
(vcondu<V:mode><VI:mode>): Ditto.
* config/riscv/riscv-protos.h (expand_vcond): Ditto.
* config/riscv/riscv-v.cc (expand_vcond): Ditto.
---
gcc/config/riscv/autovec.md | 38 ---------------------------------
gcc/config/riscv/riscv-protos.h | 1 -
gcc/config/riscv/riscv-v.cc | 22 -------------------
3 files changed, 61 deletions(-)
Comments
On Mon, 26 Jun 2023, Juzhe-Zhong wrote:
> Previously, Richi has suggested that vcond patterns are only needed when target
> support comparison + select consuming 1 instruction.
>
> Now, I do the experiments on removing those "vcond" patterns, it works perfectly.
>
> All testcases PASS.
>
> Really appreicate Richi helps us recognize such issue.
>
> Now remove all "vcond" patterns as Richi suggested.
Btw, it's also good to have a target clear of 'vcond' to verify we are
indeed happy with just vcmp and vcond_mask in the middle-end.
I see there's only a single user of vcondeq (x86), looks like an
opportunity to remove this optab ... (unless my grep skills are confused).
Thanks,
Richard.
> gcc/ChangeLog:
>
> * config/riscv/autovec.md (vcond<V:mode><VI:mode>): Remove redundant vcond patterns.
> (vcondu<V:mode><VI:mode>): Ditto.
> * config/riscv/riscv-protos.h (expand_vcond): Ditto.
> * config/riscv/riscv-v.cc (expand_vcond): Ditto.
>
> ---
> gcc/config/riscv/autovec.md | 38 ---------------------------------
> gcc/config/riscv/riscv-protos.h | 1 -
> gcc/config/riscv/riscv-v.cc | 22 -------------------
> 3 files changed, 61 deletions(-)
>
> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index 5de43a8d647..19100b5b2cb 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -311,44 +311,6 @@
> }
> )
>
> -;; -------------------------------------------------------------------------
> -;; ---- [INT,FP] Compare and select
> -;; -------------------------------------------------------------------------
> -;; The patterns in this section are synthetic.
> -;; -------------------------------------------------------------------------
> -
> -(define_expand "vcond<V:mode><VI:mode>"
> - [(set (match_operand:V 0 "register_operand")
> - (if_then_else:V
> - (match_operator 3 "comparison_operator"
> - [(match_operand:VI 4 "register_operand")
> - (match_operand:VI 5 "register_operand")])
> - (match_operand:V 1 "register_operand")
> - (match_operand:V 2 "register_operand")))]
> - "TARGET_VECTOR && known_eq (GET_MODE_NUNITS (<V:MODE>mode),
> - GET_MODE_NUNITS (<VI:MODE>mode))"
> - {
> - riscv_vector::expand_vcond (operands);
> - DONE;
> - }
> -)
> -
> -(define_expand "vcondu<V:mode><VI:mode>"
> - [(set (match_operand:V 0 "register_operand")
> - (if_then_else:V
> - (match_operator 3 "comparison_operator"
> - [(match_operand:VI 4 "register_operand")
> - (match_operand:VI 5 "register_operand")])
> - (match_operand:V 1 "register_operand")
> - (match_operand:V 2 "register_operand")))]
> - "TARGET_VECTOR && known_eq (GET_MODE_NUNITS (<V:MODE>mode),
> - GET_MODE_NUNITS (<VI:MODE>mode))"
> - {
> - riscv_vector::expand_vcond (operands);
> - DONE;
> - }
> -)
> -
> ;; -------------------------------------------------------------------------
> ;; ---- [INT] Sign and zero extension
> ;; -------------------------------------------------------------------------
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index f686edab3d1..7265b1c8401 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -252,7 +252,6 @@ machine_mode preferred_simd_mode (scalar_mode);
> opt_machine_mode get_mask_mode (machine_mode);
> void expand_vec_series (rtx, rtx, rtx);
> void expand_vec_init (rtx, rtx);
> -void expand_vcond (rtx *);
> void expand_vec_perm (rtx, rtx, rtx, rtx);
> void expand_select_vl (rtx *);
> void expand_load_store (rtx *, bool);
> diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
> index 5518394be1e..f6dd0d8e2a4 100644
> --- a/gcc/config/riscv/riscv-v.cc
> +++ b/gcc/config/riscv/riscv-v.cc
> @@ -2421,28 +2421,6 @@ expand_vec_cmp_float (rtx target, rtx_code code, rtx op0, rtx op1,
> return false;
> }
>
> -/* Expand an RVV vcond pattern with operands OPS. DATA_MODE is the mode
> - of the data being merged and CMP_MODE is the mode of the values being
> - compared. */
> -
> -void
> -expand_vcond (rtx *ops)
> -{
> - machine_mode cmp_mode = GET_MODE (ops[4]);
> - machine_mode data_mode = GET_MODE (ops[1]);
> - machine_mode mask_mode = get_mask_mode (cmp_mode).require ();
> - rtx mask = gen_reg_rtx (mask_mode);
> - if (FLOAT_MODE_P (cmp_mode))
> - {
> - if (expand_vec_cmp_float (mask, GET_CODE (ops[3]), ops[4], ops[5], true))
> - std::swap (ops[1], ops[2]);
> - }
> - else
> - expand_vec_cmp (mask, GET_CODE (ops[3]), ops[4], ops[5]);
> - emit_insn (
> - gen_vcond_mask (data_mode, data_mode, ops[0], ops[1], ops[2], mask));
> -}
> -
> /* Implement vec_perm<mode>. */
>
> void
>
ok for trunk, thanks :)
On Mon, Jun 26, 2023 at 4:44 PM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Mon, 26 Jun 2023, Juzhe-Zhong wrote:
>
> > Previously, Richi has suggested that vcond patterns are only needed when target
> > support comparison + select consuming 1 instruction.
> >
> > Now, I do the experiments on removing those "vcond" patterns, it works perfectly.
> >
> > All testcases PASS.
> >
> > Really appreicate Richi helps us recognize such issue.
> >
> > Now remove all "vcond" patterns as Richi suggested.
>
> Btw, it's also good to have a target clear of 'vcond' to verify we are
> indeed happy with just vcmp and vcond_mask in the middle-end.
>
> I see there's only a single user of vcondeq (x86), looks like an
> opportunity to remove this optab ... (unless my grep skills are confused).
>
> Thanks,
> Richard.
>
> > gcc/ChangeLog:
> >
> > * config/riscv/autovec.md (vcond<V:mode><VI:mode>): Remove redundant vcond patterns.
> > (vcondu<V:mode><VI:mode>): Ditto.
> > * config/riscv/riscv-protos.h (expand_vcond): Ditto.
> > * config/riscv/riscv-v.cc (expand_vcond): Ditto.
> >
> > ---
> > gcc/config/riscv/autovec.md | 38 ---------------------------------
> > gcc/config/riscv/riscv-protos.h | 1 -
> > gcc/config/riscv/riscv-v.cc | 22 -------------------
> > 3 files changed, 61 deletions(-)
> >
> > diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> > index 5de43a8d647..19100b5b2cb 100644
> > --- a/gcc/config/riscv/autovec.md
> > +++ b/gcc/config/riscv/autovec.md
> > @@ -311,44 +311,6 @@
> > }
> > )
> >
> > -;; -------------------------------------------------------------------------
> > -;; ---- [INT,FP] Compare and select
> > -;; -------------------------------------------------------------------------
> > -;; The patterns in this section are synthetic.
> > -;; -------------------------------------------------------------------------
> > -
> > -(define_expand "vcond<V:mode><VI:mode>"
> > - [(set (match_operand:V 0 "register_operand")
> > - (if_then_else:V
> > - (match_operator 3 "comparison_operator"
> > - [(match_operand:VI 4 "register_operand")
> > - (match_operand:VI 5 "register_operand")])
> > - (match_operand:V 1 "register_operand")
> > - (match_operand:V 2 "register_operand")))]
> > - "TARGET_VECTOR && known_eq (GET_MODE_NUNITS (<V:MODE>mode),
> > - GET_MODE_NUNITS (<VI:MODE>mode))"
> > - {
> > - riscv_vector::expand_vcond (operands);
> > - DONE;
> > - }
> > -)
> > -
> > -(define_expand "vcondu<V:mode><VI:mode>"
> > - [(set (match_operand:V 0 "register_operand")
> > - (if_then_else:V
> > - (match_operator 3 "comparison_operator"
> > - [(match_operand:VI 4 "register_operand")
> > - (match_operand:VI 5 "register_operand")])
> > - (match_operand:V 1 "register_operand")
> > - (match_operand:V 2 "register_operand")))]
> > - "TARGET_VECTOR && known_eq (GET_MODE_NUNITS (<V:MODE>mode),
> > - GET_MODE_NUNITS (<VI:MODE>mode))"
> > - {
> > - riscv_vector::expand_vcond (operands);
> > - DONE;
> > - }
> > -)
> > -
> > ;; -------------------------------------------------------------------------
> > ;; ---- [INT] Sign and zero extension
> > ;; -------------------------------------------------------------------------
> > diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> > index f686edab3d1..7265b1c8401 100644
> > --- a/gcc/config/riscv/riscv-protos.h
> > +++ b/gcc/config/riscv/riscv-protos.h
> > @@ -252,7 +252,6 @@ machine_mode preferred_simd_mode (scalar_mode);
> > opt_machine_mode get_mask_mode (machine_mode);
> > void expand_vec_series (rtx, rtx, rtx);
> > void expand_vec_init (rtx, rtx);
> > -void expand_vcond (rtx *);
> > void expand_vec_perm (rtx, rtx, rtx, rtx);
> > void expand_select_vl (rtx *);
> > void expand_load_store (rtx *, bool);
> > diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
> > index 5518394be1e..f6dd0d8e2a4 100644
> > --- a/gcc/config/riscv/riscv-v.cc
> > +++ b/gcc/config/riscv/riscv-v.cc
> > @@ -2421,28 +2421,6 @@ expand_vec_cmp_float (rtx target, rtx_code code, rtx op0, rtx op1,
> > return false;
> > }
> >
> > -/* Expand an RVV vcond pattern with operands OPS. DATA_MODE is the mode
> > - of the data being merged and CMP_MODE is the mode of the values being
> > - compared. */
> > -
> > -void
> > -expand_vcond (rtx *ops)
> > -{
> > - machine_mode cmp_mode = GET_MODE (ops[4]);
> > - machine_mode data_mode = GET_MODE (ops[1]);
> > - machine_mode mask_mode = get_mask_mode (cmp_mode).require ();
> > - rtx mask = gen_reg_rtx (mask_mode);
> > - if (FLOAT_MODE_P (cmp_mode))
> > - {
> > - if (expand_vec_cmp_float (mask, GET_CODE (ops[3]), ops[4], ops[5], true))
> > - std::swap (ops[1], ops[2]);
> > - }
> > - else
> > - expand_vec_cmp (mask, GET_CODE (ops[3]), ops[4], ops[5]);
> > - emit_insn (
> > - gen_vcond_mask (data_mode, data_mode, ops[0], ops[1], ops[2], mask));
> > -}
> > -
> > /* Implement vec_perm<mode>. */
> >
> > void
> >
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
> Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
> HRB 36809 (AG Nuernberg)
Committed, thanks kito.
Pan
-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+pan2.li=intel.com@gcc.gnu.org> On Behalf Of Kito Cheng via Gcc-patches
Sent: Monday, June 26, 2023 5:08 PM
To: Richard Biener <rguenther@suse.de>
Cc: Juzhe-Zhong <juzhe.zhong@rivai.ai>; gcc-patches@gcc.gnu.org; kito.cheng@sifive.com; palmer@dabbelt.com; palmer@rivosinc.com; jeffreyalaw@gmail.com; rdapp.gcc@gmail.com
Subject: Re: [PATCH] RISC-V: Remove redundant vcond patterns
ok for trunk, thanks :)
On Mon, Jun 26, 2023 at 4:44 PM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Mon, 26 Jun 2023, Juzhe-Zhong wrote:
>
> > Previously, Richi has suggested that vcond patterns are only needed when target
> > support comparison + select consuming 1 instruction.
> >
> > Now, I do the experiments on removing those "vcond" patterns, it works perfectly.
> >
> > All testcases PASS.
> >
> > Really appreicate Richi helps us recognize such issue.
> >
> > Now remove all "vcond" patterns as Richi suggested.
>
> Btw, it's also good to have a target clear of 'vcond' to verify we are
> indeed happy with just vcmp and vcond_mask in the middle-end.
>
> I see there's only a single user of vcondeq (x86), looks like an
> opportunity to remove this optab ... (unless my grep skills are confused).
>
> Thanks,
> Richard.
>
> > gcc/ChangeLog:
> >
> > * config/riscv/autovec.md (vcond<V:mode><VI:mode>): Remove redundant vcond patterns.
> > (vcondu<V:mode><VI:mode>): Ditto.
> > * config/riscv/riscv-protos.h (expand_vcond): Ditto.
> > * config/riscv/riscv-v.cc (expand_vcond): Ditto.
> >
> > ---
> > gcc/config/riscv/autovec.md | 38 ---------------------------------
> > gcc/config/riscv/riscv-protos.h | 1 -
> > gcc/config/riscv/riscv-v.cc | 22 -------------------
> > 3 files changed, 61 deletions(-)
> >
> > diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> > index 5de43a8d647..19100b5b2cb 100644
> > --- a/gcc/config/riscv/autovec.md
> > +++ b/gcc/config/riscv/autovec.md
> > @@ -311,44 +311,6 @@
> > }
> > )
> >
> > -;; -------------------------------------------------------------------------
> > -;; ---- [INT,FP] Compare and select
> > -;; -------------------------------------------------------------------------
> > -;; The patterns in this section are synthetic.
> > -;; -------------------------------------------------------------------------
> > -
> > -(define_expand "vcond<V:mode><VI:mode>"
> > - [(set (match_operand:V 0 "register_operand")
> > - (if_then_else:V
> > - (match_operator 3 "comparison_operator"
> > - [(match_operand:VI 4 "register_operand")
> > - (match_operand:VI 5 "register_operand")])
> > - (match_operand:V 1 "register_operand")
> > - (match_operand:V 2 "register_operand")))]
> > - "TARGET_VECTOR && known_eq (GET_MODE_NUNITS (<V:MODE>mode),
> > - GET_MODE_NUNITS (<VI:MODE>mode))"
> > - {
> > - riscv_vector::expand_vcond (operands);
> > - DONE;
> > - }
> > -)
> > -
> > -(define_expand "vcondu<V:mode><VI:mode>"
> > - [(set (match_operand:V 0 "register_operand")
> > - (if_then_else:V
> > - (match_operator 3 "comparison_operator"
> > - [(match_operand:VI 4 "register_operand")
> > - (match_operand:VI 5 "register_operand")])
> > - (match_operand:V 1 "register_operand")
> > - (match_operand:V 2 "register_operand")))]
> > - "TARGET_VECTOR && known_eq (GET_MODE_NUNITS (<V:MODE>mode),
> > - GET_MODE_NUNITS (<VI:MODE>mode))"
> > - {
> > - riscv_vector::expand_vcond (operands);
> > - DONE;
> > - }
> > -)
> > -
> > ;; -------------------------------------------------------------------------
> > ;; ---- [INT] Sign and zero extension
> > ;; -------------------------------------------------------------------------
> > diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> > index f686edab3d1..7265b1c8401 100644
> > --- a/gcc/config/riscv/riscv-protos.h
> > +++ b/gcc/config/riscv/riscv-protos.h
> > @@ -252,7 +252,6 @@ machine_mode preferred_simd_mode (scalar_mode);
> > opt_machine_mode get_mask_mode (machine_mode);
> > void expand_vec_series (rtx, rtx, rtx);
> > void expand_vec_init (rtx, rtx);
> > -void expand_vcond (rtx *);
> > void expand_vec_perm (rtx, rtx, rtx, rtx);
> > void expand_select_vl (rtx *);
> > void expand_load_store (rtx *, bool);
> > diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
> > index 5518394be1e..f6dd0d8e2a4 100644
> > --- a/gcc/config/riscv/riscv-v.cc
> > +++ b/gcc/config/riscv/riscv-v.cc
> > @@ -2421,28 +2421,6 @@ expand_vec_cmp_float (rtx target, rtx_code code, rtx op0, rtx op1,
> > return false;
> > }
> >
> > -/* Expand an RVV vcond pattern with operands OPS. DATA_MODE is the mode
> > - of the data being merged and CMP_MODE is the mode of the values being
> > - compared. */
> > -
> > -void
> > -expand_vcond (rtx *ops)
> > -{
> > - machine_mode cmp_mode = GET_MODE (ops[4]);
> > - machine_mode data_mode = GET_MODE (ops[1]);
> > - machine_mode mask_mode = get_mask_mode (cmp_mode).require ();
> > - rtx mask = gen_reg_rtx (mask_mode);
> > - if (FLOAT_MODE_P (cmp_mode))
> > - {
> > - if (expand_vec_cmp_float (mask, GET_CODE (ops[3]), ops[4], ops[5], true))
> > - std::swap (ops[1], ops[2]);
> > - }
> > - else
> > - expand_vec_cmp (mask, GET_CODE (ops[3]), ops[4], ops[5]);
> > - emit_insn (
> > - gen_vcond_mask (data_mode, data_mode, ops[0], ops[1], ops[2], mask));
> > -}
> > -
> > /* Implement vec_perm<mode>. */
> >
> > void
> >
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
> Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
> HRB 36809 (AG Nuernberg)
@@ -311,44 +311,6 @@
}
)
-;; -------------------------------------------------------------------------
-;; ---- [INT,FP] Compare and select
-;; -------------------------------------------------------------------------
-;; The patterns in this section are synthetic.
-;; -------------------------------------------------------------------------
-
-(define_expand "vcond<V:mode><VI:mode>"
- [(set (match_operand:V 0 "register_operand")
- (if_then_else:V
- (match_operator 3 "comparison_operator"
- [(match_operand:VI 4 "register_operand")
- (match_operand:VI 5 "register_operand")])
- (match_operand:V 1 "register_operand")
- (match_operand:V 2 "register_operand")))]
- "TARGET_VECTOR && known_eq (GET_MODE_NUNITS (<V:MODE>mode),
- GET_MODE_NUNITS (<VI:MODE>mode))"
- {
- riscv_vector::expand_vcond (operands);
- DONE;
- }
-)
-
-(define_expand "vcondu<V:mode><VI:mode>"
- [(set (match_operand:V 0 "register_operand")
- (if_then_else:V
- (match_operator 3 "comparison_operator"
- [(match_operand:VI 4 "register_operand")
- (match_operand:VI 5 "register_operand")])
- (match_operand:V 1 "register_operand")
- (match_operand:V 2 "register_operand")))]
- "TARGET_VECTOR && known_eq (GET_MODE_NUNITS (<V:MODE>mode),
- GET_MODE_NUNITS (<VI:MODE>mode))"
- {
- riscv_vector::expand_vcond (operands);
- DONE;
- }
-)
-
;; -------------------------------------------------------------------------
;; ---- [INT] Sign and zero extension
;; -------------------------------------------------------------------------
@@ -252,7 +252,6 @@ machine_mode preferred_simd_mode (scalar_mode);
opt_machine_mode get_mask_mode (machine_mode);
void expand_vec_series (rtx, rtx, rtx);
void expand_vec_init (rtx, rtx);
-void expand_vcond (rtx *);
void expand_vec_perm (rtx, rtx, rtx, rtx);
void expand_select_vl (rtx *);
void expand_load_store (rtx *, bool);
@@ -2421,28 +2421,6 @@ expand_vec_cmp_float (rtx target, rtx_code code, rtx op0, rtx op1,
return false;
}
-/* Expand an RVV vcond pattern with operands OPS. DATA_MODE is the mode
- of the data being merged and CMP_MODE is the mode of the values being
- compared. */
-
-void
-expand_vcond (rtx *ops)
-{
- machine_mode cmp_mode = GET_MODE (ops[4]);
- machine_mode data_mode = GET_MODE (ops[1]);
- machine_mode mask_mode = get_mask_mode (cmp_mode).require ();
- rtx mask = gen_reg_rtx (mask_mode);
- if (FLOAT_MODE_P (cmp_mode))
- {
- if (expand_vec_cmp_float (mask, GET_CODE (ops[3]), ops[4], ops[5], true))
- std::swap (ops[1], ops[2]);
- }
- else
- expand_vec_cmp (mask, GET_CODE (ops[3]), ops[4], ops[5]);
- emit_insn (
- gen_vcond_mask (data_mode, data_mode, ops[0], ops[1], ops[2], mask));
-}
-
/* Implement vec_perm<mode>. */
void