RE: [PATCH v4] DSE: Fix ICE after allow vector type in get_stored_val
Committed, thanks Jeff. Let's wait for a while before backporting. Pan -Original Message- From: Jeff Law Sent: Monday, May 20, 2024 12:23 AM To: Li, Pan2 ; gcc-patches@gcc.gnu.org Cc: juzhe.zh...@rivai.ai; kito.ch...@gmail.com; Liu, Hongtao ; richard.guent...@gmail.com Subject: Re: [PATCH v4] DSE: Fix ICE after allow vector type in get_stored_val On 5/2/24 7:51 PM, pan2...@intel.com wrote: > From: Pan Li > > We allowed vector type for get_stored_val when read is less than or > equal to store in previous. Unfortunately, the valididate_subreg > treats the vector type's size is less than vector register as > invalid. Then we will have ICE here. > > This patch would like to fix it by filter-out the invalid type size, > and make sure the subreg is valid for both the read_mode and store_mode > before perform the real gen_lowpart. > > The below test suites are passed for this patch: > > * The x86 bootstrap test. > * The x86 regression test. > * The riscv rv64gcv regression test. > * The riscv rv64gc regression test. > * The aarch64 regression test. > > gcc/ChangeLog: > > * dse.cc (get_stored_val): Make sure read_mode/write_mode > is valid subreg before gen_lowpart. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/rvv/base/bug-6.c: New test. OK for the trunk. Let's let it simmer on the trunk for a while before we consider backporting. jeff
Re: [PATCH v4] DSE: Fix ICE after allow vector type in get_stored_val
On 5/2/24 7:51 PM, pan2...@intel.com wrote: From: Pan Li We allowed vector type for get_stored_val when read is less than or equal to store in previous. Unfortunately, the valididate_subreg treats the vector type's size is less than vector register as invalid. Then we will have ICE here. This patch would like to fix it by filter-out the invalid type size, and make sure the subreg is valid for both the read_mode and store_mode before perform the real gen_lowpart. The below test suites are passed for this patch: * The x86 bootstrap test. * The x86 regression test. * The riscv rv64gcv regression test. * The riscv rv64gc regression test. * The aarch64 regression test. gcc/ChangeLog: * dse.cc (get_stored_val): Make sure read_mode/write_mode is valid subreg before gen_lowpart. gcc/testsuite/ChangeLog: * gcc.target/riscv/rvv/base/bug-6.c: New test. OK for the trunk. Let's let it simmer on the trunk for a while before we consider backporting. jeff
RE: [PATCH v4] DSE: Fix ICE after allow vector type in get_stored_val
Kindly ping, looks no build error from Linaro for arm.
Pan
-Original Message-
From: Li, Pan2
Sent: Friday, May 3, 2024 9:52 AM
To: gcc-patches@gcc.gnu.org
Cc: jeffreya...@gmail.com; juzhe.zh...@rivai.ai; kito.ch...@gmail.com; Liu,
Hongtao ; richard.guent...@gmail.com; Li, Pan2
Subject: [PATCH v4] DSE: Fix ICE after allow vector type in get_stored_val
From: Pan Li
We allowed vector type for get_stored_val when read is less than or
equal to store in previous. Unfortunately, the valididate_subreg
treats the vector type's size is less than vector register as
invalid. Then we will have ICE here.
This patch would like to fix it by filter-out the invalid type size,
and make sure the subreg is valid for both the read_mode and store_mode
before perform the real gen_lowpart.
The below test suites are passed for this patch:
* The x86 bootstrap test.
* The x86 regression test.
* The riscv rv64gcv regression test.
* The riscv rv64gc regression test.
* The aarch64 regression test.
gcc/ChangeLog:
* dse.cc (get_stored_val): Make sure read_mode/write_mode
is valid subreg before gen_lowpart.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/bug-6.c: New test.
Signed-off-by: Pan Li
---
gcc/dse.cc| 4 +++-
.../gcc.target/riscv/rvv/base/bug-6.c | 22 +++
2 files changed, 25 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
diff --git a/gcc/dse.cc b/gcc/dse.cc
index edc7a1dfecf..1596da91da0 100644
--- a/gcc/dse.cc
+++ b/gcc/dse.cc
@@ -1946,7 +1946,9 @@ get_stored_val (store_info *store_info, machine_mode
read_mode,
copy_rtx (store_info->const_rhs));
else if (VECTOR_MODE_P (read_mode) && VECTOR_MODE_P (store_mode)
&& known_le (GET_MODE_BITSIZE (read_mode), GET_MODE_BITSIZE (store_mode))
-&& targetm.modes_tieable_p (read_mode, store_mode))
+&& targetm.modes_tieable_p (read_mode, store_mode)
+&& validate_subreg (read_mode, store_mode, copy_rtx (store_info->rhs),
+ subreg_lowpart_offset (read_mode, store_mode)))
read_reg = gen_lowpart (read_mode, copy_rtx (store_info->rhs));
else
read_reg = extract_low_bits (read_mode, store_mode,
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
new file mode 100644
index 000..5bb00b8f587
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
@@ -0,0 +1,22 @@
+/* Test that we do not have ice when compile */
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -ftree-vectorize" } */
+
+struct A { float x, y; };
+struct B { struct A u; };
+
+extern void bar (struct A *);
+
+float
+f3 (struct B *x, int y)
+{
+ struct A p = {1.0f, 2.0f};
+ struct A *q = [y].u;
+
+ __builtin_memcpy (>x, , sizeof (float));
+ __builtin_memcpy (>y, , sizeof (float));
+
+ bar ();
+
+ return x[y].u.x + x[y].u.y;
+}
--
2.34.1
[PATCH v4] DSE: Fix ICE after allow vector type in get_stored_val
From: Pan Li
We allowed vector type for get_stored_val when read is less than or
equal to store in previous. Unfortunately, the valididate_subreg
treats the vector type's size is less than vector register as
invalid. Then we will have ICE here.
This patch would like to fix it by filter-out the invalid type size,
and make sure the subreg is valid for both the read_mode and store_mode
before perform the real gen_lowpart.
The below test suites are passed for this patch:
* The x86 bootstrap test.
* The x86 regression test.
* The riscv rv64gcv regression test.
* The riscv rv64gc regression test.
* The aarch64 regression test.
gcc/ChangeLog:
* dse.cc (get_stored_val): Make sure read_mode/write_mode
is valid subreg before gen_lowpart.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/bug-6.c: New test.
Signed-off-by: Pan Li
---
gcc/dse.cc| 4 +++-
.../gcc.target/riscv/rvv/base/bug-6.c | 22 +++
2 files changed, 25 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
diff --git a/gcc/dse.cc b/gcc/dse.cc
index edc7a1dfecf..1596da91da0 100644
--- a/gcc/dse.cc
+++ b/gcc/dse.cc
@@ -1946,7 +1946,9 @@ get_stored_val (store_info *store_info, machine_mode
read_mode,
copy_rtx (store_info->const_rhs));
else if (VECTOR_MODE_P (read_mode) && VECTOR_MODE_P (store_mode)
&& known_le (GET_MODE_BITSIZE (read_mode), GET_MODE_BITSIZE (store_mode))
-&& targetm.modes_tieable_p (read_mode, store_mode))
+&& targetm.modes_tieable_p (read_mode, store_mode)
+&& validate_subreg (read_mode, store_mode, copy_rtx (store_info->rhs),
+ subreg_lowpart_offset (read_mode, store_mode)))
read_reg = gen_lowpart (read_mode, copy_rtx (store_info->rhs));
else
read_reg = extract_low_bits (read_mode, store_mode,
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
new file mode 100644
index 000..5bb00b8f587
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
@@ -0,0 +1,22 @@
+/* Test that we do not have ice when compile */
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -ftree-vectorize" } */
+
+struct A { float x, y; };
+struct B { struct A u; };
+
+extern void bar (struct A *);
+
+float
+f3 (struct B *x, int y)
+{
+ struct A p = {1.0f, 2.0f};
+ struct A *q = [y].u;
+
+ __builtin_memcpy (>x, , sizeof (float));
+ __builtin_memcpy (>y, , sizeof (float));
+
+ bar ();
+
+ return x[y].u.x + x[y].u.y;
+}
--
2.34.1
