Re: [PATCH] Middle-end: Change order of LEN_MASK_LOAD/LEN_MASK_STORE arguments

2023-07-03 Thread Richard Sandiford via Gcc-patches
juzhe.zh...@rivai.ai writes:
> From: Ju-Zhe Zhong 
>
> Hi, Richard and Richi.
>
> According to Richard's review comments:
> https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623405.html
>
> current len, bias and mask order is not reasonable.
>
> Change {len,mask,bias} into {len,bias,mask}.
>
> This patch is a simple fix patch, ok for trunk ?

Sorry to be awkward, but can you put the patches in the opposite order,
so that this one comes first, and then the gather/scatter patch comes
second?

I don't think we want internal_fn_bias_index.  The point of reordering
the arguments was to put the bias immediately after the length in all
cases.  In other words, the length and the bias form a pair: the bias
only exists if the length does, and the length only exists if the bias
does.

In other respects the patch LGTM.  Thanks for doing this.

Richard

>
> gcc/ChangeLog:
>
> * config/riscv/autovec.md: Change order of 
> LEN_MASK_LOAD/LEN_MASK_STORE arguments.
> * config/riscv/riscv-v.cc (expand_load_store): Ditto.
> * doc/md.texi: Ditto.
> * gimple-fold.cc (gimple_fold_partial_load_store_mem_ref): Ditto.
> * internal-fn.cc (len_maskload_direct): Ditto.
> (len_maskstore_direct): Ditto.
> (add_len_bias_and_mask_args): Ditto.
> (expand_partial_load_optab_fn): Ditto.
> (expand_partial_store_optab_fn): Ditto.
> (internal_fn_mask_index): Ditto.
> (internal_fn_len_index): Ditto.
> (internal_fn_bias_index): Ditto.
> (internal_fn_stored_value_index): Ditto.
> (internal_len_load_store_bias): Ditto.
> * tree-ssa-dse.cc (initialize_ao_ref_for_dse): Ditto.
> * tree-vect-stmts.cc (vectorizable_store): Ditto.
> (vectorizable_load): Ditto.
>
> ---
>  gcc/config/riscv/autovec.md |   8 +-
>  gcc/config/riscv/riscv-v.cc |   2 +-
>  gcc/doc/md.texi |  16 ++--
>  gcc/gimple-fold.cc  |   7 +-
>  gcc/internal-fn.cc  | 176 +---
>  gcc/tree-ssa-dse.cc |  13 +--
>  gcc/tree-vect-stmts.cc  |   6 +-
>  7 files changed, 90 insertions(+), 138 deletions(-)
>
> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index 1488f2be1be..4ab0e9f99eb 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -26,8 +26,8 @@
>[(match_operand:V 0 "register_operand")
> (match_operand:V 1 "memory_operand")
> (match_operand 2 "autovec_length_operand")
> -   (match_operand: 3 "vector_mask_operand")
> -   (match_operand 4 "const_0_operand")]
> +   (match_operand 3 "const_0_operand")
> +   (match_operand: 4 "vector_mask_operand")]
>"TARGET_VECTOR"
>  {
>riscv_vector::expand_load_store (operands, true);
> @@ -38,8 +38,8 @@
>[(match_operand:V 0 "memory_operand")
> (match_operand:V 1 "register_operand")
> (match_operand 2 "autovec_length_operand")
> -   (match_operand: 3 "vector_mask_operand")
> -   (match_operand 4 "const_0_operand")]
> +   (match_operand 3 "const_0_operand")
> +   (match_operand: 4 "vector_mask_operand")]
>"TARGET_VECTOR"
>  {
>riscv_vector::expand_load_store (operands, false);
> diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
> index adb8d7d36a5..8d5bed7ebe4 100644
> --- a/gcc/config/riscv/riscv-v.cc
> +++ b/gcc/config/riscv/riscv-v.cc
> @@ -2777,7 +2777,7 @@ expand_load_store (rtx *ops, bool is_load)
>  {
>poly_int64 value;
>rtx len = ops[2];
> -  rtx mask = ops[3];
> +  rtx mask = ops[4];
>machine_mode mode = GET_MODE (ops[0]);
>  
>if (poly_int_rtx_p (len, &value) && known_eq (value, GET_MODE_NUNITS 
> (mode)))
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index b44d1ba3af9..f14dd32b2dc 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -5319,15 +5319,15 @@ This pattern is not allowed to @code{FAIL}.
>  @cindex @code{len_maskload@var{m}@var{n}} instruction pattern
>  @item @samp{len_maskload@var{m}@var{n}}
>  Perform a masked load from the memory location pointed to by operand 1
> -into register operand 0.  (operand 2 + operand 4) elements are loaded from
> +into register operand 0.  (operand 2 + operand 3) elements are loaded from
>  memory and other elements in operand 0 are set to undefined values.
>  This is a combination of len_load and maskload.
>  Operands 0 and 1 have mode @var{m}, which must be a vector mode.  Operand 2
>  has whichever integer mode the target prefers.  A mask is specified in
> -operand 3 which must be of type @var{n}.  The mask has lower precedence than
> +operand 4 which must be of type @var{n}.  The mask has lower precedence than
>  the length and is itself subject to length masking,
> -i.e. only mask indices < (operand 2 + operand 4) are used.
> -Operand 4 conceptually has mode @code{QI}.
> +i.e. only mask indices < (operand 2 + operand 3) are used.
> +Operand 3 conceptually has mode @code{QI}.
>  
>  Operand 2 can be a variable or a constant amount.  Operand 4 specifies a
>

[PATCH] Middle-end: Change order of LEN_MASK_LOAD/LEN_MASK_STORE arguments

2023-07-02 Thread juzhe . zhong
From: Ju-Zhe Zhong 

Hi, Richard and Richi.

According to Richard's review comments:
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623405.html

current len, bias and mask order is not reasonable.

Change {len,mask,bias} into {len,bias,mask}.

This patch is a simple fix patch, ok for trunk ?

gcc/ChangeLog:

* config/riscv/autovec.md: Change order of LEN_MASK_LOAD/LEN_MASK_STORE 
arguments.
* config/riscv/riscv-v.cc (expand_load_store): Ditto.
* doc/md.texi: Ditto.
* gimple-fold.cc (gimple_fold_partial_load_store_mem_ref): Ditto.
* internal-fn.cc (len_maskload_direct): Ditto.
(len_maskstore_direct): Ditto.
(add_len_bias_and_mask_args): Ditto.
(expand_partial_load_optab_fn): Ditto.
(expand_partial_store_optab_fn): Ditto.
(internal_fn_mask_index): Ditto.
(internal_fn_len_index): Ditto.
(internal_fn_bias_index): Ditto.
(internal_fn_stored_value_index): Ditto.
(internal_len_load_store_bias): Ditto.
* tree-ssa-dse.cc (initialize_ao_ref_for_dse): Ditto.
* tree-vect-stmts.cc (vectorizable_store): Ditto.
(vectorizable_load): Ditto.

---
 gcc/config/riscv/autovec.md |   8 +-
 gcc/config/riscv/riscv-v.cc |   2 +-
 gcc/doc/md.texi |  16 ++--
 gcc/gimple-fold.cc  |   7 +-
 gcc/internal-fn.cc  | 176 +---
 gcc/tree-ssa-dse.cc |  13 +--
 gcc/tree-vect-stmts.cc  |   6 +-
 7 files changed, 90 insertions(+), 138 deletions(-)

diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index 1488f2be1be..4ab0e9f99eb 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -26,8 +26,8 @@
   [(match_operand:V 0 "register_operand")
(match_operand:V 1 "memory_operand")
(match_operand 2 "autovec_length_operand")
-   (match_operand: 3 "vector_mask_operand")
-   (match_operand 4 "const_0_operand")]
+   (match_operand 3 "const_0_operand")
+   (match_operand: 4 "vector_mask_operand")]
   "TARGET_VECTOR"
 {
   riscv_vector::expand_load_store (operands, true);
@@ -38,8 +38,8 @@
   [(match_operand:V 0 "memory_operand")
(match_operand:V 1 "register_operand")
(match_operand 2 "autovec_length_operand")
-   (match_operand: 3 "vector_mask_operand")
-   (match_operand 4 "const_0_operand")]
+   (match_operand 3 "const_0_operand")
+   (match_operand: 4 "vector_mask_operand")]
   "TARGET_VECTOR"
 {
   riscv_vector::expand_load_store (operands, false);
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index adb8d7d36a5..8d5bed7ebe4 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -2777,7 +2777,7 @@ expand_load_store (rtx *ops, bool is_load)
 {
   poly_int64 value;
   rtx len = ops[2];
-  rtx mask = ops[3];
+  rtx mask = ops[4];
   machine_mode mode = GET_MODE (ops[0]);
 
   if (poly_int_rtx_p (len, &value) && known_eq (value, GET_MODE_NUNITS (mode)))
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index b44d1ba3af9..f14dd32b2dc 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5319,15 +5319,15 @@ This pattern is not allowed to @code{FAIL}.
 @cindex @code{len_maskload@var{m}@var{n}} instruction pattern
 @item @samp{len_maskload@var{m}@var{n}}
 Perform a masked load from the memory location pointed to by operand 1
-into register operand 0.  (operand 2 + operand 4) elements are loaded from
+into register operand 0.  (operand 2 + operand 3) elements are loaded from
 memory and other elements in operand 0 are set to undefined values.
 This is a combination of len_load and maskload.
 Operands 0 and 1 have mode @var{m}, which must be a vector mode.  Operand 2
 has whichever integer mode the target prefers.  A mask is specified in
-operand 3 which must be of type @var{n}.  The mask has lower precedence than
+operand 4 which must be of type @var{n}.  The mask has lower precedence than
 the length and is itself subject to length masking,
-i.e. only mask indices < (operand 2 + operand 4) are used.
-Operand 4 conceptually has mode @code{QI}.
+i.e. only mask indices < (operand 2 + operand 3) are used.
+Operand 3 conceptually has mode @code{QI}.
 
 Operand 2 can be a variable or a constant amount.  Operand 4 specifies a
 constant bias: it is either a constant 0 or a constant -1.  The predicate on
@@ -5346,14 +5346,14 @@ This pattern is not allowed to @code{FAIL}.
 @cindex @code{len_maskstore@var{m}@var{n}} instruction pattern
 @item @samp{len_maskstore@var{m}@var{n}}
 Perform a masked store from vector register operand 1 into memory operand 0.
-(operand 2 + operand 4) elements are stored to memory
+(operand 2 + operand 3) elements are stored to memory
 and leave the other elements of operand 0 unchanged.
 This is a combination of len_store and maskstore.
 Operands 0 and 1 have mode @var{m}, which must be a vector mode.  Operand 2 
has whichever
-integer mode the target prefers.  A mask is specified in operand 3 which must 
be
+integer mode the target