[v3] DSE: Fix ICE after allow vector type in get_stored_val
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
fail
|
Testing failed
|
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
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 size is greater
than or equal to the vector register size before gen_lowpart.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/pr111720-10.c: Adjust asm checker.
* 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 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/pr111720-10.c | 2 +-
3 files changed, 26 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
Comments
Linaro reports one build failure for arm but works well in aarch64.
/home/tcwg-build/workspace/tcwg_gnu_2/abe/snapshots/gcc.git~master/gcc/dse.cc:1951:45: error: 'REGMODE_NATURAL_SIZE' was not declared in this scope
Looks only part of backend implemented REGMODE_NATURAL_SIZE, then reference this macro here may not be a good idea.
gcc/config/i386/i386.cc:20835:/* Implement REGMODE_NATURAL_SIZE(MODE). */
gcc/config/i386/i386.h:1050:#define REGMODE_NATURAL_SIZE(MODE) ix86_regmode_natural_size (MODE)
gcc/config/aarch64/aarch64.cc:2479:/* Implement REGMODE_NATURAL_SIZE. */
gcc/config/aarch64/aarch64.h:1505:#define REGMODE_NATURAL_SIZE(MODE) aarch64_regmode_natural_size (MODE)
gcc/config/riscv/riscv.h:1212:#define REGMODE_NATURAL_SIZE(MODE) riscv_regmode_natural_size (MODE)
gcc/config/riscv/riscv.cc:10241:/* Implement REGMODE_NATURAL_SIZE. */
gcc/config/sparc/sparc.h:706:#define REGMODE_NATURAL_SIZE(MODE) sparc_regmode_natural_size (MODE)
Pan
-----Original Message-----
From: Li, Pan2 <pan2.li@intel.com>
Sent: Tuesday, April 30, 2024 3:17 PM
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 v3] 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 size is greater
than or equal to the vector register size before gen_lowpart.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/pr111720-10.c: Adjust asm checker.
* 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 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/pr111720-10.c | 2 +-
3 files changed, 26 insertions(+), 2 deletions(-)
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..258d2ccc299 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)
+ /* It's invalid in validate_subreg if read_mode size is < reg natural. */
+ && known_ge (GET_MODE_SIZE (read_mode), REGMODE_NATURAL_SIZE (read_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;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr111720-10.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr111720-10.c
index 215eb99ce0f..ee6b2ccf7ad 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/base/pr111720-10.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr111720-10.c
@@ -15,4 +15,4 @@ vbool4_t test () {
}
/* { dg-final { scan-assembler-not {vle[0-9]+\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} } } */
-/* { dg-final { scan-assembler-not {vs[0-9]+r\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} } } */
+/* { dg-final { scan-assembler-times {vs[0-9]+r\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} 1 } } */
Try to invoke validate_subreg directly in v4 as below.
https://gcc.gnu.org/pipermail/gcc-patches/2024-May/650596.html
Pan
-----Original Message-----
From: Li, Pan2
Sent: Tuesday, April 30, 2024 7:36 PM
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
Subject: RE: [PATCH v3] DSE: Fix ICE after allow vector type in get_stored_val
Linaro reports one build failure for arm but works well in aarch64.
/home/tcwg-build/workspace/tcwg_gnu_2/abe/snapshots/gcc.git~master/gcc/dse.cc:1951:45: error: 'REGMODE_NATURAL_SIZE' was not declared in this scope
Looks only part of backend implemented REGMODE_NATURAL_SIZE, then reference this macro here may not be a good idea.
gcc/config/i386/i386.cc:20835:/* Implement REGMODE_NATURAL_SIZE(MODE). */
gcc/config/i386/i386.h:1050:#define REGMODE_NATURAL_SIZE(MODE) ix86_regmode_natural_size (MODE)
gcc/config/aarch64/aarch64.cc:2479:/* Implement REGMODE_NATURAL_SIZE. */
gcc/config/aarch64/aarch64.h:1505:#define REGMODE_NATURAL_SIZE(MODE) aarch64_regmode_natural_size (MODE)
gcc/config/riscv/riscv.h:1212:#define REGMODE_NATURAL_SIZE(MODE) riscv_regmode_natural_size (MODE)
gcc/config/riscv/riscv.cc:10241:/* Implement REGMODE_NATURAL_SIZE. */
gcc/config/sparc/sparc.h:706:#define REGMODE_NATURAL_SIZE(MODE) sparc_regmode_natural_size (MODE)
Pan
-----Original Message-----
From: Li, Pan2 <pan2.li@intel.com>
Sent: Tuesday, April 30, 2024 3:17 PM
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 v3] 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 size is greater
than or equal to the vector register size before gen_lowpart.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/pr111720-10.c: Adjust asm checker.
* 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 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/pr111720-10.c | 2 +-
3 files changed, 26 insertions(+), 2 deletions(-)
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..258d2ccc299 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)
+ /* It's invalid in validate_subreg if read_mode size is < reg natural. */
+ && known_ge (GET_MODE_SIZE (read_mode), REGMODE_NATURAL_SIZE (read_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;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr111720-10.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr111720-10.c
index 215eb99ce0f..ee6b2ccf7ad 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/base/pr111720-10.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr111720-10.c
@@ -15,4 +15,4 @@ vbool4_t test () {
}
/* { dg-final { scan-assembler-not {vle[0-9]+\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} } } */
-/* { dg-final { scan-assembler-not {vs[0-9]+r\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} } } */
+/* { dg-final { scan-assembler-times {vs[0-9]+r\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} 1 } } */
@@ -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)
+ /* It's invalid in validate_subreg if read_mode size is < reg natural. */
+ && known_ge (GET_MODE_SIZE (read_mode), REGMODE_NATURAL_SIZE (read_mode)))
read_reg = gen_lowpart (read_mode, copy_rtx (store_info->rhs));
else
read_reg = extract_low_bits (read_mode, store_mode,
new file mode 100644
@@ -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;
+}
@@ -15,4 +15,4 @@ vbool4_t test () {
}
/* { dg-final { scan-assembler-not {vle[0-9]+\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} } } */
-/* { dg-final { scan-assembler-not {vs[0-9]+r\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} } } */
+/* { dg-final { scan-assembler-times {vs[0-9]+r\.v\s+v[0-9]+,\s*[0-9]+\(sp\)} 1 } } */