diff mbox series

[v4] DSE: Fix ICE after allow vector type in get_stored_val

Message ID 20240503015135.1400555-1-pan2.li@intel.com
State New
Headers show
Series [v4] DSE: Fix ICE after allow vector type in get_stored_val | expand

Commit Message

Li, Pan2 May 3, 2024, 1:51 a.m. UTC
From: Pan Li <pan2.li@intel.com>

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 <pan2.li@intel.com>
---
 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

Comments

Li, Pan2 May 16, 2024, 4:06 a.m. UTC | #1
Kindly ping, looks no build error from Linaro for arm.

Pan

-----Original Message-----
From: Li, Pan2 <pan2.li@intel.com> 
Sent: Friday, May 3, 2024 9:52 AM
To: gcc-patches@gcc.gnu.org
Cc: jeffreyalaw@gmail.com; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; Liu, Hongtao <hongtao.liu@intel.com>; richard.guenther@gmail.com; Li, Pan2 <pan2.li@intel.com>
Subject: [PATCH v4] DSE: Fix ICE after allow vector type in get_stored_val

From: Pan Li <pan2.li@intel.com>

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 <pan2.li@intel.com>
---
 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 00000000000..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 = &x[y].u;
+
+  __builtin_memcpy (&q->x, &p.x, sizeof (float));
+  __builtin_memcpy (&q->y, &p.y, sizeof (float));
+
+  bar (&p);
+
+  return x[y].u.x + x[y].u.y;
+}
Jeff Law May 19, 2024, 4:23 p.m. UTC | #2
On 5/2/24 7:51 PM, pan2.li@intel.com wrote:
> From: Pan Li <pan2.li@intel.com>
> 
> 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
Li, Pan2 May 20, 2024, 1:08 a.m. UTC | #3
Committed, thanks Jeff. Let's wait for a while before backporting.

Pan

-----Original Message-----
From: Jeff Law <jeffreyalaw@gmail.com> 
Sent: Monday, May 20, 2024 12:23 AM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
Cc: juzhe.zhong@rivai.ai; kito.cheng@gmail.com; Liu, Hongtao <hongtao.liu@intel.com>; richard.guenther@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.li@intel.com wrote:
> From: Pan Li <pan2.li@intel.com>
> 
> 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
diff mbox series

Patch

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 00000000000..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 = &x[y].u;
+
+  __builtin_memcpy (&q->x, &p.x, sizeof (float));
+  __builtin_memcpy (&q->y, &p.y, sizeof (float));
+
+  bar (&p);
+
+  return x[y].u.x + x[y].u.y;
+}