RISC-V: Fix incorrect index(offset) of gather/scatter

Message ID 20231011090309.2987108-1-juzhe.zhong@rivai.ai
State Unresolved
Headers
Series RISC-V: Fix incorrect index(offset) of gather/scatter |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

juzhe.zhong@rivai.ai Oct. 11, 2023, 9:03 a.m. UTC
  I suddenly I made a mistake that was lucky un-exposed.

https://godbolt.org/z/c3jzrh7or

GCC is using 32 bit index offset:

        vsll.vi v1,v1,2
        vsetvli zero,a5,e32,m1,ta,ma
        vluxei32.v      v1,(a1),v1

This is wrong since v1 may overflow 32bit after vsll.vi.

After this patch:

vsext.vf2	v8,v4
vsll.vi	v8,v8,2
vluxei64.v	v8,(a1),v8

Same as Clang.

Regression passed. Ok for trunk ?

gcc/ChangeLog:

	* config/riscv/autovec.md: Fix offset bug.
	* config/riscv/riscv-protos.h (gather_scatter_valid_offset_p): New function.
	* config/riscv/riscv-v.cc (expand_gather_scatter): Fix offset bug.
	(gather_scatter_valid_offset_p): New function.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c: New test.

---
 gcc/config/riscv/autovec.md                   | 28 +++++++++----------
 gcc/config/riscv/riscv-protos.h               |  1 +
 gcc/config/riscv/riscv-v.cc                   | 16 +++++++++--
 .../autovec/gather-scatter/offset_extend-1.c  | 14 ++++++++++
 4 files changed, 42 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c
  

Comments

juzhe.zhong@rivai.ai Oct. 11, 2023, 9:37 a.m. UTC | #1
Refine the codes in V2:
https://gcc.gnu.org/pipermail/gcc-patches/2023-October/632619.html 



juzhe.zhong@rivai.ai
 
From: Juzhe-Zhong
Date: 2023-10-11 17:03
To: gcc-patches
CC: kito.cheng; kito.cheng; jeffreyalaw; rdapp.gcc; Juzhe-Zhong
Subject: [PATCH] RISC-V: Fix incorrect index(offset) of gather/scatter
I suddenly I made a mistake that was lucky un-exposed.
 
https://godbolt.org/z/c3jzrh7or
 
GCC is using 32 bit index offset:
 
        vsll.vi v1,v1,2
        vsetvli zero,a5,e32,m1,ta,ma
        vluxei32.v      v1,(a1),v1
 
This is wrong since v1 may overflow 32bit after vsll.vi.
 
After this patch:
 
vsext.vf2 v8,v4
vsll.vi v8,v8,2
vluxei64.v v8,(a1),v8
 
Same as Clang.
 
Regression passed. Ok for trunk ?
 
gcc/ChangeLog:
 
* config/riscv/autovec.md: Fix offset bug.
* config/riscv/riscv-protos.h (gather_scatter_valid_offset_p): New function.
* config/riscv/riscv-v.cc (expand_gather_scatter): Fix offset bug.
(gather_scatter_valid_offset_p): New function.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c: New test.
 
---
gcc/config/riscv/autovec.md                   | 28 +++++++++----------
gcc/config/riscv/riscv-protos.h               |  1 +
gcc/config/riscv/riscv-v.cc                   | 16 +++++++++--
.../autovec/gather-scatter/offset_extend-1.c  | 14 ++++++++++
4 files changed, 42 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c
 
diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index 41bff3a318f..07607bff71e 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -59,7 +59,7 @@
    (match_operand:<RATIO64:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO64:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -74,7 +74,7 @@
    (match_operand:<RATIO32:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO32:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -89,7 +89,7 @@
    (match_operand:<RATIO16:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO16:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -104,7 +104,7 @@
    (match_operand:<RATIO8:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO8:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -119,7 +119,7 @@
    (match_operand:<RATIO4:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO4:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -134,7 +134,7 @@
    (match_operand:<RATIO2:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO2:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -153,7 +153,7 @@
    (match_operand:<RATIO1:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO1:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -172,7 +172,7 @@
    (match_operand:<RATIO64:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO64:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -187,7 +187,7 @@
    (match_operand:<RATIO32:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO32:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -202,7 +202,7 @@
    (match_operand:<RATIO16:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO16:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -217,7 +217,7 @@
    (match_operand:<RATIO8:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO8:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -232,7 +232,7 @@
    (match_operand:<RATIO4:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO4:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -247,7 +247,7 @@
    (match_operand:<RATIO2:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO2:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -266,7 +266,7 @@
    (match_operand:<RATIO1:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO1:MODE>mode)"
{
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 43426a5326b..f805332f378 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -543,6 +543,7 @@ opt_machine_mode vectorize_related_mode (machine_mode, scalar_mode,
unsigned int autovectorize_vector_modes (vec<machine_mode> *, bool);
bool cmp_lmul_le_one (machine_mode);
bool cmp_lmul_gt_one (machine_mode);
+bool gather_scatter_valid_offset_p (machine_mode);
}
/* We classify builtin types into two classes:
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index c72e411f125..6e90f1bbee1 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -3491,15 +3491,14 @@ expand_gather_scatter (rtx *ops, bool is_load)
   machine_mode vec_mode = GET_MODE (vec_reg);
   machine_mode idx_mode = GET_MODE (vec_offset);
-  scalar_mode inner_vec_mode = GET_MODE_INNER (vec_mode);
   scalar_mode inner_idx_mode = GET_MODE_INNER (idx_mode);
-  unsigned inner_vsize = GET_MODE_BITSIZE (inner_vec_mode);
   unsigned inner_offsize = GET_MODE_BITSIZE (inner_idx_mode);
   poly_int64 nunits = GET_MODE_NUNITS (vec_mode);
   poly_int64 value;
   bool is_vlmax = poly_int_rtx_p (len, &value) && known_eq (value, nunits);
-  if (inner_offsize < inner_vsize)
+  /* Extend the offset element to address width.  */
+  if (inner_offsize < GET_MODE_BITSIZE (GET_MODE (ptr)).to_constant ())
     {
       /* 7.2. Vector Load/Store Addressing Modes.
If the vector offset elements are narrower than XLEN, they are
@@ -3796,6 +3795,17 @@ cmp_lmul_gt_one (machine_mode mode)
   return false;
}
+/* Return true if the gather/scatter offset is valid.  */
+bool
+gather_scatter_valid_offset_p (machine_mode offset_mode)
+{
+  machine_mode new_offset_mode;
+  if (get_vector_mode (Pmode, GET_MODE_NUNITS (offset_mode))
+ .exists (&new_offset_mode))
+    return true;
+  return false;
+}
+
/* We don't have to convert the floating point to integer when the
    mantissa is zero.  Thus, ther will be a limitation for both the
    single and double precision floating point.  There will be no
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c
new file mode 100644
index 00000000000..b7936eb749a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-march=rv64gcv -mabi=lp64d" } */
+
+void
+f (int *restrict y, int *restrict x, int *restrict indices, int n)
+{
+  for (int i = 0; i < n; ++i)
+    y[i] = x[indices[i]] + 1;
+}
+
+/* { dg-final { scan-assembler {vluxei64\.v} } } */
+/* { dg-final { scan-assembler {vsll\.vi} } } */
+/* { dg-final { scan-assembler {vsext\.vf2} } } */
+/* { dg-final { scan-assembler-not {vluxei32\.v} } } */
-- 
2.36.3
  

Patch

diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index 41bff3a318f..07607bff71e 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -59,7 +59,7 @@ 
    (match_operand:<RATIO64:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO64:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -74,7 +74,7 @@ 
    (match_operand:<RATIO32:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO32:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -89,7 +89,7 @@ 
    (match_operand:<RATIO16:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO16:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -104,7 +104,7 @@ 
    (match_operand:<RATIO8:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO8:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -119,7 +119,7 @@ 
    (match_operand:<RATIO4:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO4:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -134,7 +134,7 @@ 
    (match_operand:<RATIO2:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO2:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -153,7 +153,7 @@ 
    (match_operand:<RATIO1:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO1:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, true);
   DONE;
@@ -172,7 +172,7 @@ 
    (match_operand:<RATIO64:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO64:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -187,7 +187,7 @@ 
    (match_operand:<RATIO32:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO32:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -202,7 +202,7 @@ 
    (match_operand:<RATIO16:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO16:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -217,7 +217,7 @@ 
    (match_operand:<RATIO8:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO8:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -232,7 +232,7 @@ 
    (match_operand:<RATIO4:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO4:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -247,7 +247,7 @@ 
    (match_operand:<RATIO2:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO2:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
@@ -266,7 +266,7 @@ 
    (match_operand:<RATIO1:VM> 5 "vector_mask_operand")
    (match_operand 6 "autovec_length_operand")
    (match_operand 7 "const_0_operand")]
-  "TARGET_VECTOR"
+  "TARGET_VECTOR && riscv_vector::gather_scatter_valid_offset_p (<RATIO1:MODE>mode)"
 {
   riscv_vector::expand_gather_scatter (operands, false);
   DONE;
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 43426a5326b..f805332f378 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -543,6 +543,7 @@  opt_machine_mode vectorize_related_mode (machine_mode, scalar_mode,
 unsigned int autovectorize_vector_modes (vec<machine_mode> *, bool);
 bool cmp_lmul_le_one (machine_mode);
 bool cmp_lmul_gt_one (machine_mode);
+bool gather_scatter_valid_offset_p (machine_mode);
 }
 
 /* We classify builtin types into two classes:
diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
index c72e411f125..6e90f1bbee1 100644
--- a/gcc/config/riscv/riscv-v.cc
+++ b/gcc/config/riscv/riscv-v.cc
@@ -3491,15 +3491,14 @@  expand_gather_scatter (rtx *ops, bool is_load)
 
   machine_mode vec_mode = GET_MODE (vec_reg);
   machine_mode idx_mode = GET_MODE (vec_offset);
-  scalar_mode inner_vec_mode = GET_MODE_INNER (vec_mode);
   scalar_mode inner_idx_mode = GET_MODE_INNER (idx_mode);
-  unsigned inner_vsize = GET_MODE_BITSIZE (inner_vec_mode);
   unsigned inner_offsize = GET_MODE_BITSIZE (inner_idx_mode);
   poly_int64 nunits = GET_MODE_NUNITS (vec_mode);
   poly_int64 value;
   bool is_vlmax = poly_int_rtx_p (len, &value) && known_eq (value, nunits);
 
-  if (inner_offsize < inner_vsize)
+  /* Extend the offset element to address width.  */
+  if (inner_offsize < GET_MODE_BITSIZE (GET_MODE (ptr)).to_constant ())
     {
       /* 7.2. Vector Load/Store Addressing Modes.
 	 If the vector offset elements are narrower than XLEN, they are
@@ -3796,6 +3795,17 @@  cmp_lmul_gt_one (machine_mode mode)
   return false;
 }
 
+/* Return true if the gather/scatter offset is valid.  */
+bool
+gather_scatter_valid_offset_p (machine_mode offset_mode)
+{
+  machine_mode new_offset_mode;
+  if (get_vector_mode (Pmode, GET_MODE_NUNITS (offset_mode))
+	.exists (&new_offset_mode))
+    return true;
+  return false;
+}
+
 /* We don't have to convert the floating point to integer when the
    mantissa is zero.  Thus, ther will be a limitation for both the
    single and double precision floating point.  There will be no
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c
new file mode 100644
index 00000000000..b7936eb749a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/gather-scatter/offset_extend-1.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-march=rv64gcv -mabi=lp64d" } */
+
+void
+f (int *restrict y, int *restrict x, int *restrict indices, int n)
+{
+  for (int i = 0; i < n; ++i)
+    y[i] = x[indices[i]] + 1;
+}
+
+/* { dg-final { scan-assembler {vluxei64\.v} } } */
+/* { dg-final { scan-assembler {vsll\.vi} } } */
+/* { dg-final { scan-assembler {vsext\.vf2} } } */
+/* { dg-final { scan-assembler-not {vluxei32\.v} } } */