* [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
@ 2024-03-18 6:05 pan2.li
2024-03-18 6:21 ` juzhe.zhong
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: pan2.li @ 2024-03-18 6:05 UTC (permalink / raw)
To: gcc-patches; +Cc: juzhe.zhong, kito.cheng, yanzhang.wang, Pan Li
From: Pan Li <pan2.li@intel.com>
This patch would like to fix one ICE for __attribute__((target("arch=+v"))
and likewise extension(s). Given we have sample code as below:
void __attribute__((target("arch=+v")))
test_2 (int *a, int *b, int *out, unsigned count)
{
unsigned i;
for (i = 0; i < count; i++)
out[i] = a[i] + b[i];
}
It will have ICE when build with -march=rv64gc -O3.
test.c: In function ‘test_2’:
test.c:4:1: internal compiler error: Floating point exception
4 | {
| ^
0x1a5891b crash_signal
.../__RISC-V_BUILD__/../gcc/toplev.cc:319
0x7f0a7884251f ???
./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
0x1f51ba4 riscv_hard_regno_nregs
.../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:8143
0x1967bb9 init_reg_modes_target()
.../__RISC-V_BUILD__/../gcc/reginfo.cc:471
0x13fc029 init_emit_regs()
.../__RISC-V_BUILD__/../gcc/emit-rtl.cc:6237
0x1a5b83d target_reinit()
.../__RISC-V_BUILD__/../gcc/toplev.cc:1936
0x35e374d save_target_globals()
.../__RISC-V_BUILD__/../gcc/target-globals.cc:92
0x35e381f save_target_globals_default_opts()
.../__RISC-V_BUILD__/../gcc/target-globals.cc:122
0x1f544cc riscv_save_restore_target_globals(tree_node*)
.../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:9138
0x1f55c36 riscv_set_current_function
...
There are two reasons for this ICE.
1. The implied extension(s) of v are not well handled and the
TARGET_MIN_VLEN is 0 which is not reinitialized. Then the
size / TARGET_MIN_VLEN will have DivideByZero.
2. The machine modes of the vector types will be vary after
the v extension is introduced.
This patch passed below testsuite:
1. The riscv fully regression test.
PR target/114352
gcc/ChangeLog:
* common/config/riscv/riscv-common.cc
(riscv_subset_list::parse_single_ext): Add implied, combine
and conflict check after parse single extension.
* config/riscv/riscv.cc (riscv_set_current_function):
Reini the machine mode before when set cur function.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/pr114352-1.c: New test.
* gcc.target/riscv/rvv/base/pr114352-2.c: New test.
Signed-off-by: Pan Li <pan2.li@intel.com>
---
gcc/common/config/riscv/riscv-common.cc | 33 ++++++++---
gcc/config/riscv/riscv.cc | 4 ++
.../gcc.target/riscv/rvv/base/pr114352-1.c | 58 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/pr114352-2.c | 27 +++++++++
4 files changed, 115 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 48efef40dfd..d32bf147eca 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -1375,20 +1375,39 @@ riscv_subset_list::parse_single_multiletter_ext (const char *p,
const char *
riscv_subset_list::parse_single_ext (const char *p, bool exact_single_p)
{
+ const char *end_of_ext;
+
switch (p[0])
{
case 'x':
- return parse_single_multiletter_ext (p, "x", "non-standard extension",
- exact_single_p);
+ end_of_ext = parse_single_multiletter_ext (p, "x",
+ "non-standard extension",
+ exact_single_p);
+ break;
case 'z':
- return parse_single_multiletter_ext (p, "z", "sub-extension",
- exact_single_p);
+ end_of_ext = parse_single_multiletter_ext (p, "z", "sub-extension",
+ exact_single_p);
+ break;
case 's':
- return parse_single_multiletter_ext (p, "s", "supervisor extension",
- exact_single_p);
+ end_of_ext = parse_single_multiletter_ext (p, "s", "supervisor extension",
+ exact_single_p);
+ break;
default:
- return parse_single_std_ext (p, exact_single_p);
+ end_of_ext = parse_single_std_ext (p, exact_single_p);
+ break;
}
+
+ /* Make sure the implied or combined extension is included after add
+ a new std extension to subset list. For exmaple as below,
+
+ void __attribute__((target("arch=+v"))) func () with -march=rv64gc.
+
+ The implied zvl128b and zve64d of the std v should be included. */
+ handle_implied_ext (p);
+ handle_combine_ext ();
+ check_conflict_ext ();
+
+ return end_of_ext;
}
/* Parsing arch string to subset list, return NULL if parsing failed. */
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 680c4a728e9..89acb94af10 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -9474,6 +9474,10 @@ riscv_set_current_function (tree decl)
cl_target_option_restore (&global_options, &global_options_set,
TREE_TARGET_OPTION (new_tree));
+ /* The ISA extension can vary based on the function extension like target.
+ Thus, make sure that the machine modes are reflected correctly here. */
+ init_adjust_machine_modes ();
+
riscv_save_restore_target_globals (new_tree);
}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
new file mode 100644
index 00000000000..b3f1f20fb79
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
@@ -0,0 +1,58 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc -mabi=lp64 -O3 -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/*
+** test_1:
+** beq\s+a3,\s*zero,\s*\.L[0-9]+
+** ...
+** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
+** ...
+** ret
+*/
+void
+test_1 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/*
+** test_2:
+** ...
+** vsetvli\s+[atx][0-9]+,\s*[atx][0-9]+,\s*e32,\s*m1,\s*ta,\s*ma
+** ...
+** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
+** ...
+*/
+void
+__attribute__((target("arch=+v")))
+test_2 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/*
+** test_3:
+** beq\s+a3,\s*zero,\s*\.L[0-9]+
+** ...
+** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
+** ...
+** ret
+*/
+void
+test_3 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0\"" } } */
+/* { dg-final { scan-assembler ".option arch, rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicsr2p0_zifencei2p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
new file mode 100644
index 00000000000..3b3d69d2751
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
@@ -0,0 +1,27 @@
+/* Test that we do not have ice when compile */
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc -mabi=lp64 -O3" } */
+
+#define DEF_ATTR_FUNC(ATTR, ID) \
+void ATTR \
+test_##ID (int *a, int *b, int *out, unsigned count) \
+{ \
+ unsigned i; \
+ \
+ for (i = 0; i < count; i++) \
+ out[i] = a[i] + b[i]; \
+}
+
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve32x"))), 1)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve32f"))), 2)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64x"))), 3)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64f"))), 4)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64d"))), 5)
+DEF_ATTR_FUNC (__attribute__((target("arch=+v"))), 6)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl64b"))), 7)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl128b"))), 8)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl256b"))), 9)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl512b"))), 10)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl1024b"))), 11)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl2048b"))), 12)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl4096b"))), 13)
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
2024-03-18 6:05 [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v")) pan2.li
@ 2024-03-18 6:21 ` juzhe.zhong
2024-03-21 12:38 ` Kito Cheng
2024-03-22 1:12 ` [PATCH v2] " pan2.li
2 siblings, 0 replies; 8+ messages in thread
From: juzhe.zhong @ 2024-03-18 6:21 UTC (permalink / raw)
To: pan2.li, gcc-patches; +Cc: kito.cheng, yanzhang.wang, pan2.li
[-- Attachment #1: Type: text/plain, Size: 8794 bytes --]
I can't review this stuff. Let kito review this.
Thanks.
juzhe.zhong@rivai.ai
From: pan2.li
Date: 2024-03-18 14:05
To: gcc-patches
CC: juzhe.zhong; kito.cheng; yanzhang.wang; Pan Li
Subject: [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
From: Pan Li <pan2.li@intel.com>
This patch would like to fix one ICE for __attribute__((target("arch=+v"))
and likewise extension(s). Given we have sample code as below:
void __attribute__((target("arch=+v")))
test_2 (int *a, int *b, int *out, unsigned count)
{
unsigned i;
for (i = 0; i < count; i++)
out[i] = a[i] + b[i];
}
It will have ICE when build with -march=rv64gc -O3.
test.c: In function ‘test_2’:
test.c:4:1: internal compiler error: Floating point exception
4 | {
| ^
0x1a5891b crash_signal
.../__RISC-V_BUILD__/../gcc/toplev.cc:319
0x7f0a7884251f ???
./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
0x1f51ba4 riscv_hard_regno_nregs
.../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:8143
0x1967bb9 init_reg_modes_target()
.../__RISC-V_BUILD__/../gcc/reginfo.cc:471
0x13fc029 init_emit_regs()
.../__RISC-V_BUILD__/../gcc/emit-rtl.cc:6237
0x1a5b83d target_reinit()
.../__RISC-V_BUILD__/../gcc/toplev.cc:1936
0x35e374d save_target_globals()
.../__RISC-V_BUILD__/../gcc/target-globals.cc:92
0x35e381f save_target_globals_default_opts()
.../__RISC-V_BUILD__/../gcc/target-globals.cc:122
0x1f544cc riscv_save_restore_target_globals(tree_node*)
.../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:9138
0x1f55c36 riscv_set_current_function
...
There are two reasons for this ICE.
1. The implied extension(s) of v are not well handled and the
TARGET_MIN_VLEN is 0 which is not reinitialized. Then the
size / TARGET_MIN_VLEN will have DivideByZero.
2. The machine modes of the vector types will be vary after
the v extension is introduced.
This patch passed below testsuite:
1. The riscv fully regression test.
PR target/114352
gcc/ChangeLog:
* common/config/riscv/riscv-common.cc
(riscv_subset_list::parse_single_ext): Add implied, combine
and conflict check after parse single extension.
* config/riscv/riscv.cc (riscv_set_current_function):
Reini the machine mode before when set cur function.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/pr114352-1.c: New test.
* gcc.target/riscv/rvv/base/pr114352-2.c: New test.
Signed-off-by: Pan Li <pan2.li@intel.com>
---
gcc/common/config/riscv/riscv-common.cc | 33 ++++++++---
gcc/config/riscv/riscv.cc | 4 ++
.../gcc.target/riscv/rvv/base/pr114352-1.c | 58 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/pr114352-2.c | 27 +++++++++
4 files changed, 115 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 48efef40dfd..d32bf147eca 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -1375,20 +1375,39 @@ riscv_subset_list::parse_single_multiletter_ext (const char *p,
const char *
riscv_subset_list::parse_single_ext (const char *p, bool exact_single_p)
{
+ const char *end_of_ext;
+
switch (p[0])
{
case 'x':
- return parse_single_multiletter_ext (p, "x", "non-standard extension",
- exact_single_p);
+ end_of_ext = parse_single_multiletter_ext (p, "x",
+ "non-standard extension",
+ exact_single_p);
+ break;
case 'z':
- return parse_single_multiletter_ext (p, "z", "sub-extension",
- exact_single_p);
+ end_of_ext = parse_single_multiletter_ext (p, "z", "sub-extension",
+ exact_single_p);
+ break;
case 's':
- return parse_single_multiletter_ext (p, "s", "supervisor extension",
- exact_single_p);
+ end_of_ext = parse_single_multiletter_ext (p, "s", "supervisor extension",
+ exact_single_p);
+ break;
default:
- return parse_single_std_ext (p, exact_single_p);
+ end_of_ext = parse_single_std_ext (p, exact_single_p);
+ break;
}
+
+ /* Make sure the implied or combined extension is included after add
+ a new std extension to subset list. For exmaple as below,
+
+ void __attribute__((target("arch=+v"))) func () with -march=rv64gc.
+
+ The implied zvl128b and zve64d of the std v should be included. */
+ handle_implied_ext (p);
+ handle_combine_ext ();
+ check_conflict_ext ();
+
+ return end_of_ext;
}
/* Parsing arch string to subset list, return NULL if parsing failed. */
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 680c4a728e9..89acb94af10 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -9474,6 +9474,10 @@ riscv_set_current_function (tree decl)
cl_target_option_restore (&global_options, &global_options_set,
TREE_TARGET_OPTION (new_tree));
+ /* The ISA extension can vary based on the function extension like target.
+ Thus, make sure that the machine modes are reflected correctly here. */
+ init_adjust_machine_modes ();
+
riscv_save_restore_target_globals (new_tree);
}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
new file mode 100644
index 00000000000..b3f1f20fb79
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
@@ -0,0 +1,58 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc -mabi=lp64 -O3 -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/*
+** test_1:
+** beq\s+a3,\s*zero,\s*\.L[0-9]+
+** ...
+** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
+** ...
+** ret
+*/
+void
+test_1 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/*
+** test_2:
+** ...
+** vsetvli\s+[atx][0-9]+,\s*[atx][0-9]+,\s*e32,\s*m1,\s*ta,\s*ma
+** ...
+** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
+** ...
+*/
+void
+__attribute__((target("arch=+v")))
+test_2 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/*
+** test_3:
+** beq\s+a3,\s*zero,\s*\.L[0-9]+
+** ...
+** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
+** ...
+** ret
+*/
+void
+test_3 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0\"" } } */
+/* { dg-final { scan-assembler ".option arch, rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicsr2p0_zifencei2p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
new file mode 100644
index 00000000000..3b3d69d2751
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
@@ -0,0 +1,27 @@
+/* Test that we do not have ice when compile */
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc -mabi=lp64 -O3" } */
+
+#define DEF_ATTR_FUNC(ATTR, ID) \
+void ATTR \
+test_##ID (int *a, int *b, int *out, unsigned count) \
+{ \
+ unsigned i; \
+ \
+ for (i = 0; i < count; i++) \
+ out[i] = a[i] + b[i]; \
+}
+
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve32x"))), 1)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve32f"))), 2)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64x"))), 3)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64f"))), 4)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64d"))), 5)
+DEF_ATTR_FUNC (__attribute__((target("arch=+v"))), 6)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl64b"))), 7)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl128b"))), 8)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl256b"))), 9)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl512b"))), 10)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl1024b"))), 11)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl2048b"))), 12)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl4096b"))), 13)
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
2024-03-18 6:05 [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v")) pan2.li
2024-03-18 6:21 ` juzhe.zhong
@ 2024-03-21 12:38 ` Kito Cheng
2024-03-21 14:06 ` Li, Pan2
2024-03-22 1:12 ` [PATCH v2] " pan2.li
2 siblings, 1 reply; 8+ messages in thread
From: Kito Cheng @ 2024-03-21 12:38 UTC (permalink / raw)
To: pan2.li; +Cc: gcc-patches, juzhe.zhong, yanzhang.wang
> +
> + /* Make sure the implied or combined extension is included after add
> + a new std extension to subset list. For exmaple as below,
> +
> + void __attribute__((target("arch=+v"))) func () with -march=rv64gc.
> +
> + The implied zvl128b and zve64d of the std v should be included. */
> + handle_implied_ext (p);
> + handle_combine_ext ();
> + check_conflict_ext ();
Extract those 3 function calls to a public function
riscv_subset_list::finalize(),
and then call that at riscv_target_attr_parser::parse_arch rather than here.
> +
> + return end_of_ext;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
2024-03-21 12:38 ` Kito Cheng
@ 2024-03-21 14:06 ` Li, Pan2
0 siblings, 0 replies; 8+ messages in thread
From: Li, Pan2 @ 2024-03-21 14:06 UTC (permalink / raw)
To: Kito Cheng; +Cc: gcc-patches, juzhe.zhong, Wang, Yanzhang
Thanks Kito, will send v2 for this change.
Pan
-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com>
Sent: Thursday, March 21, 2024 8:39 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
> +
> + /* Make sure the implied or combined extension is included after add
> + a new std extension to subset list. For exmaple as below,
> +
> + void __attribute__((target("arch=+v"))) func () with -march=rv64gc.
> +
> + The implied zvl128b and zve64d of the std v should be included. */
> + handle_implied_ext (p);
> + handle_combine_ext ();
> + check_conflict_ext ();
Extract those 3 function calls to a public function
riscv_subset_list::finalize(),
and then call that at riscv_target_attr_parser::parse_arch rather than here.
> +
> + return end_of_ext;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
2024-03-18 6:05 [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v")) pan2.li
2024-03-18 6:21 ` juzhe.zhong
2024-03-21 12:38 ` Kito Cheng
@ 2024-03-22 1:12 ` pan2.li
2024-03-22 2:24 ` Kito Cheng
2024-03-30 13:20 ` Andreas Schwab
2 siblings, 2 replies; 8+ messages in thread
From: pan2.li @ 2024-03-22 1:12 UTC (permalink / raw)
To: gcc-patches; +Cc: juzhe.zhong, kito.cheng, yanzhang.wang, Pan Li
From: Pan Li <pan2.li@intel.com>
This patch would like to fix one ICE for __attribute__((target("arch=+v"))
and likewise extension(s). Given we have sample code as below:
void __attribute__((target("arch=+v")))
test_2 (int *a, int *b, int *out, unsigned count)
{
unsigned i;
for (i = 0; i < count; i++)
out[i] = a[i] + b[i];
}
It will have ICE when build with -march=rv64gc -O3.
test.c: In function ‘test_2’:
test.c:4:1: internal compiler error: Floating point exception
4 | {
| ^
0x1a5891b crash_signal
.../__RISC-V_BUILD__/../gcc/toplev.cc:319
0x7f0a7884251f ???
./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
0x1f51ba4 riscv_hard_regno_nregs
.../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:8143
0x1967bb9 init_reg_modes_target()
.../__RISC-V_BUILD__/../gcc/reginfo.cc:471
0x13fc029 init_emit_regs()
.../__RISC-V_BUILD__/../gcc/emit-rtl.cc:6237
0x1a5b83d target_reinit()
.../__RISC-V_BUILD__/../gcc/toplev.cc:1936
0x35e374d save_target_globals()
.../__RISC-V_BUILD__/../gcc/target-globals.cc:92
0x35e381f save_target_globals_default_opts()
.../__RISC-V_BUILD__/../gcc/target-globals.cc:122
0x1f544cc riscv_save_restore_target_globals(tree_node*)
.../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:9138
0x1f55c36 riscv_set_current_function
...
There are two reasons for this ICE.
1. The implied extension(s) of v are not well handled and the
TARGET_MIN_VLEN is 0 which is not reinitialized. Then the
size / TARGET_MIN_VLEN will have DivideByZero.
2. The machine modes of the vector types will be vary after
the v extension is introduced.
This patch passed below testsuite:
1. The riscv fully regression test.
PR target/114352
gcc/ChangeLog:
* common/config/riscv/riscv-common.cc (riscv_subset_list::parse):
Replace implied, combine and check to func finalize.
(riscv_subset_list::finalize): New func impl to take care of
implied, combine ext and related checks.
* config/riscv/riscv-subset.h: Add func decl for finalize.
* config/riscv/riscv-target-attr.cc (riscv_target_attr_parser::parse_arch):
Finalize the ext before return succeed.
* config/riscv/riscv.cc (riscv_set_current_function): Reinit the
machine mode before when set cur function.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/pr114352-1.c: New test.
* gcc.target/riscv/rvv/base/pr114352-2.c: New test.
Signed-off-by: Pan Li <pan2.li@intel.com>
---
gcc/common/config/riscv/riscv-common.cc | 31 ++++++----
gcc/config/riscv/riscv-subset.h | 2 +
gcc/config/riscv/riscv-target-attr.cc | 2 +
gcc/config/riscv/riscv.cc | 4 ++
.../gcc.target/riscv/rvv/base/pr114352-1.c | 58 +++++++++++++++++++
.../gcc.target/riscv/rvv/base/pr114352-2.c | 27 +++++++++
6 files changed, 114 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 440127a2af0..15d44245b3c 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -1428,16 +1428,7 @@ riscv_subset_list::parse (const char *arch, location_t loc)
if (p == NULL)
goto fail;
- for (itr = subset_list->m_head; itr != NULL; itr = itr->next)
- {
- subset_list->handle_implied_ext (itr->name.c_str ());
- }
-
- /* Make sure all implied extensions are included. */
- gcc_assert (subset_list->check_implied_ext ());
-
- subset_list->handle_combine_ext ();
- subset_list->check_conflict_ext ();
+ subset_list->finalize ();
return subset_list;
@@ -1467,6 +1458,26 @@ riscv_subset_list::set_loc (location_t loc)
m_loc = loc;
}
+/* Make sure the implied or combined extension is included after add
+ a new std extension to subset list or likewise. For exmaple as below,
+
+ void __attribute__((target("arch=+v"))) func () with -march=rv64gc.
+
+ The implied zvl128b and zve64d of the std v should be included. */
+void
+riscv_subset_list::finalize ()
+{
+ riscv_subset_t *subset;
+
+ for (subset = m_head; subset != NULL; subset = subset->next)
+ handle_implied_ext (subset->name.c_str ());
+
+ gcc_assert (check_implied_ext ());
+
+ handle_combine_ext ();
+ check_conflict_ext ();
+}
+
/* Return the current arch string. */
std::string
diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
index ae849e2a302..ec979040e8c 100644
--- a/gcc/config/riscv/riscv-subset.h
+++ b/gcc/config/riscv/riscv-subset.h
@@ -105,6 +105,8 @@ public:
int match_score (riscv_subset_list *) const;
void set_loc (location_t);
+
+ void finalize ();
};
extern const riscv_subset_list *riscv_current_subset_list (void);
diff --git a/gcc/config/riscv/riscv-target-attr.cc b/gcc/config/riscv/riscv-target-attr.cc
index 9dbb78f28cc..37645adbb20 100644
--- a/gcc/config/riscv/riscv-target-attr.cc
+++ b/gcc/config/riscv/riscv-target-attr.cc
@@ -136,6 +136,8 @@ riscv_target_attr_parser::parse_arch (const char *str)
}
token = strtok_r (NULL, ",", &str_to_check);
}
+
+ m_subset_list->finalize ();
return true;
}
fail:
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 02a927f96b4..742974aec33 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -9492,6 +9492,10 @@ riscv_set_current_function (tree decl)
cl_target_option_restore (&global_options, &global_options_set,
TREE_TARGET_OPTION (new_tree));
+ /* The ISA extension can vary based on the function extension like target.
+ Thus, make sure that the machine modes are reflected correctly here. */
+ init_adjust_machine_modes ();
+
riscv_save_restore_target_globals (new_tree);
}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
new file mode 100644
index 00000000000..b3f1f20fb79
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
@@ -0,0 +1,58 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc -mabi=lp64 -O3 -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/*
+** test_1:
+** beq\s+a3,\s*zero,\s*\.L[0-9]+
+** ...
+** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
+** ...
+** ret
+*/
+void
+test_1 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/*
+** test_2:
+** ...
+** vsetvli\s+[atx][0-9]+,\s*[atx][0-9]+,\s*e32,\s*m1,\s*ta,\s*ma
+** ...
+** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
+** ...
+*/
+void
+__attribute__((target("arch=+v")))
+test_2 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/*
+** test_3:
+** beq\s+a3,\s*zero,\s*\.L[0-9]+
+** ...
+** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
+** ...
+** ret
+*/
+void
+test_3 (int *a, int *b, int *out, unsigned count)
+{
+ unsigned i;
+
+ for (i = 0; i < count; i++)
+ out[i] = a[i] + b[i];
+}
+
+/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0\"" } } */
+/* { dg-final { scan-assembler ".option arch, rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicsr2p0_zifencei2p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
new file mode 100644
index 00000000000..3b3d69d2751
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
@@ -0,0 +1,27 @@
+/* Test that we do not have ice when compile */
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc -mabi=lp64 -O3" } */
+
+#define DEF_ATTR_FUNC(ATTR, ID) \
+void ATTR \
+test_##ID (int *a, int *b, int *out, unsigned count) \
+{ \
+ unsigned i; \
+ \
+ for (i = 0; i < count; i++) \
+ out[i] = a[i] + b[i]; \
+}
+
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve32x"))), 1)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve32f"))), 2)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64x"))), 3)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64f"))), 4)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zve64d"))), 5)
+DEF_ATTR_FUNC (__attribute__((target("arch=+v"))), 6)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl64b"))), 7)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl128b"))), 8)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl256b"))), 9)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl512b"))), 10)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl1024b"))), 11)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl2048b"))), 12)
+DEF_ATTR_FUNC (__attribute__((target("arch=+zvl4096b"))), 13)
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
2024-03-22 1:12 ` [PATCH v2] " pan2.li
@ 2024-03-22 2:24 ` Kito Cheng
2024-03-22 2:38 ` Li, Pan2
2024-03-30 13:20 ` Andreas Schwab
1 sibling, 1 reply; 8+ messages in thread
From: Kito Cheng @ 2024-03-22 2:24 UTC (permalink / raw)
To: pan2.li; +Cc: gcc-patches, juzhe.zhong, yanzhang.wang
LGTM, thanks :)
On Fri, Mar 22, 2024 at 9:13 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> This patch would like to fix one ICE for __attribute__((target("arch=+v"))
> and likewise extension(s). Given we have sample code as below:
>
> void __attribute__((target("arch=+v")))
> test_2 (int *a, int *b, int *out, unsigned count)
> {
> unsigned i;
> for (i = 0; i < count; i++)
> out[i] = a[i] + b[i];
> }
>
> It will have ICE when build with -march=rv64gc -O3.
>
> test.c: In function ‘test_2’:
> test.c:4:1: internal compiler error: Floating point exception
> 4 | {
> | ^
> 0x1a5891b crash_signal
> .../__RISC-V_BUILD__/../gcc/toplev.cc:319
> 0x7f0a7884251f ???
> ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
> 0x1f51ba4 riscv_hard_regno_nregs
> .../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:8143
> 0x1967bb9 init_reg_modes_target()
> .../__RISC-V_BUILD__/../gcc/reginfo.cc:471
> 0x13fc029 init_emit_regs()
> .../__RISC-V_BUILD__/../gcc/emit-rtl.cc:6237
> 0x1a5b83d target_reinit()
> .../__RISC-V_BUILD__/../gcc/toplev.cc:1936
> 0x35e374d save_target_globals()
> .../__RISC-V_BUILD__/../gcc/target-globals.cc:92
> 0x35e381f save_target_globals_default_opts()
> .../__RISC-V_BUILD__/../gcc/target-globals.cc:122
> 0x1f544cc riscv_save_restore_target_globals(tree_node*)
> .../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:9138
> 0x1f55c36 riscv_set_current_function
> ...
>
> There are two reasons for this ICE.
> 1. The implied extension(s) of v are not well handled and the
> TARGET_MIN_VLEN is 0 which is not reinitialized. Then the
> size / TARGET_MIN_VLEN will have DivideByZero.
> 2. The machine modes of the vector types will be vary after
> the v extension is introduced.
>
> This patch passed below testsuite:
> 1. The riscv fully regression test.
>
> PR target/114352
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc (riscv_subset_list::parse):
> Replace implied, combine and check to func finalize.
> (riscv_subset_list::finalize): New func impl to take care of
> implied, combine ext and related checks.
> * config/riscv/riscv-subset.h: Add func decl for finalize.
> * config/riscv/riscv-target-attr.cc (riscv_target_attr_parser::parse_arch):
> Finalize the ext before return succeed.
> * config/riscv/riscv.cc (riscv_set_current_function): Reinit the
> machine mode before when set cur function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/base/pr114352-1.c: New test.
> * gcc.target/riscv/rvv/base/pr114352-2.c: New test.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/common/config/riscv/riscv-common.cc | 31 ++++++----
> gcc/config/riscv/riscv-subset.h | 2 +
> gcc/config/riscv/riscv-target-attr.cc | 2 +
> gcc/config/riscv/riscv.cc | 4 ++
> .../gcc.target/riscv/rvv/base/pr114352-1.c | 58 +++++++++++++++++++
> .../gcc.target/riscv/rvv/base/pr114352-2.c | 27 +++++++++
> 6 files changed, 114 insertions(+), 10 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> index 440127a2af0..15d44245b3c 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -1428,16 +1428,7 @@ riscv_subset_list::parse (const char *arch, location_t loc)
> if (p == NULL)
> goto fail;
>
> - for (itr = subset_list->m_head; itr != NULL; itr = itr->next)
> - {
> - subset_list->handle_implied_ext (itr->name.c_str ());
> - }
> -
> - /* Make sure all implied extensions are included. */
> - gcc_assert (subset_list->check_implied_ext ());
> -
> - subset_list->handle_combine_ext ();
> - subset_list->check_conflict_ext ();
> + subset_list->finalize ();
>
> return subset_list;
>
> @@ -1467,6 +1458,26 @@ riscv_subset_list::set_loc (location_t loc)
> m_loc = loc;
> }
>
> +/* Make sure the implied or combined extension is included after add
> + a new std extension to subset list or likewise. For exmaple as below,
> +
> + void __attribute__((target("arch=+v"))) func () with -march=rv64gc.
> +
> + The implied zvl128b and zve64d of the std v should be included. */
> +void
> +riscv_subset_list::finalize ()
> +{
> + riscv_subset_t *subset;
> +
> + for (subset = m_head; subset != NULL; subset = subset->next)
> + handle_implied_ext (subset->name.c_str ());
> +
> + gcc_assert (check_implied_ext ());
> +
> + handle_combine_ext ();
> + check_conflict_ext ();
> +}
> +
> /* Return the current arch string. */
>
> std::string
> diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
> index ae849e2a302..ec979040e8c 100644
> --- a/gcc/config/riscv/riscv-subset.h
> +++ b/gcc/config/riscv/riscv-subset.h
> @@ -105,6 +105,8 @@ public:
> int match_score (riscv_subset_list *) const;
>
> void set_loc (location_t);
> +
> + void finalize ();
> };
>
> extern const riscv_subset_list *riscv_current_subset_list (void);
> diff --git a/gcc/config/riscv/riscv-target-attr.cc b/gcc/config/riscv/riscv-target-attr.cc
> index 9dbb78f28cc..37645adbb20 100644
> --- a/gcc/config/riscv/riscv-target-attr.cc
> +++ b/gcc/config/riscv/riscv-target-attr.cc
> @@ -136,6 +136,8 @@ riscv_target_attr_parser::parse_arch (const char *str)
> }
> token = strtok_r (NULL, ",", &str_to_check);
> }
> +
> + m_subset_list->finalize ();
> return true;
> }
> fail:
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 02a927f96b4..742974aec33 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -9492,6 +9492,10 @@ riscv_set_current_function (tree decl)
> cl_target_option_restore (&global_options, &global_options_set,
> TREE_TARGET_OPTION (new_tree));
>
> + /* The ISA extension can vary based on the function extension like target.
> + Thus, make sure that the machine modes are reflected correctly here. */
> + init_adjust_machine_modes ();
> +
> riscv_save_restore_target_globals (new_tree);
> }
>
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> new file mode 100644
> index 00000000000..b3f1f20fb79
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> @@ -0,0 +1,58 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64 -O3 -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_1:
> +** beq\s+a3,\s*zero,\s*\.L[0-9]+
> +** ...
> +** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
> +** ...
> +** ret
> +*/
> +void
> +test_1 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/*
> +** test_2:
> +** ...
> +** vsetvli\s+[atx][0-9]+,\s*[atx][0-9]+,\s*e32,\s*m1,\s*ta,\s*ma
> +** ...
> +** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
> +** ...
> +*/
> +void
> +__attribute__((target("arch=+v")))
> +test_2 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/*
> +** test_3:
> +** beq\s+a3,\s*zero,\s*\.L[0-9]+
> +** ...
> +** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
> +** ...
> +** ret
> +*/
> +void
> +test_3 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0\"" } } */
> +/* { dg-final { scan-assembler ".option arch, rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicsr2p0_zifencei2p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
> new file mode 100644
> index 00000000000..3b3d69d2751
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
> @@ -0,0 +1,27 @@
> +/* Test that we do not have ice when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64 -O3" } */
> +
> +#define DEF_ATTR_FUNC(ATTR, ID) \
> +void ATTR \
> +test_##ID (int *a, int *b, int *out, unsigned count) \
> +{ \
> + unsigned i; \
> + \
> + for (i = 0; i < count; i++) \
> + out[i] = a[i] + b[i]; \
> +}
> +
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve32x"))), 1)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve32f"))), 2)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64x"))), 3)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64f"))), 4)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64d"))), 5)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+v"))), 6)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl64b"))), 7)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl128b"))), 8)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl256b"))), 9)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl512b"))), 10)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl1024b"))), 11)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl2048b"))), 12)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl4096b"))), 13)
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread* RE: [PATCH v2] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
2024-03-22 2:24 ` Kito Cheng
@ 2024-03-22 2:38 ` Li, Pan2
0 siblings, 0 replies; 8+ messages in thread
From: Li, Pan2 @ 2024-03-22 2:38 UTC (permalink / raw)
To: Kito Cheng; +Cc: gcc-patches, juzhe.zhong, Wang, Yanzhang
Committed, thanks Kito.
Pan
-----Original Message-----
From: Kito Cheng <kito.cheng@gmail.com>
Sent: Friday, March 22, 2024 10:24 AM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: [PATCH v2] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
LGTM, thanks :)
On Fri, Mar 22, 2024 at 9:13 AM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> This patch would like to fix one ICE for __attribute__((target("arch=+v"))
> and likewise extension(s). Given we have sample code as below:
>
> void __attribute__((target("arch=+v")))
> test_2 (int *a, int *b, int *out, unsigned count)
> {
> unsigned i;
> for (i = 0; i < count; i++)
> out[i] = a[i] + b[i];
> }
>
> It will have ICE when build with -march=rv64gc -O3.
>
> test.c: In function ‘test_2’:
> test.c:4:1: internal compiler error: Floating point exception
> 4 | {
> | ^
> 0x1a5891b crash_signal
> .../__RISC-V_BUILD__/../gcc/toplev.cc:319
> 0x7f0a7884251f ???
> ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
> 0x1f51ba4 riscv_hard_regno_nregs
> .../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:8143
> 0x1967bb9 init_reg_modes_target()
> .../__RISC-V_BUILD__/../gcc/reginfo.cc:471
> 0x13fc029 init_emit_regs()
> .../__RISC-V_BUILD__/../gcc/emit-rtl.cc:6237
> 0x1a5b83d target_reinit()
> .../__RISC-V_BUILD__/../gcc/toplev.cc:1936
> 0x35e374d save_target_globals()
> .../__RISC-V_BUILD__/../gcc/target-globals.cc:92
> 0x35e381f save_target_globals_default_opts()
> .../__RISC-V_BUILD__/../gcc/target-globals.cc:122
> 0x1f544cc riscv_save_restore_target_globals(tree_node*)
> .../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:9138
> 0x1f55c36 riscv_set_current_function
> ...
>
> There are two reasons for this ICE.
> 1. The implied extension(s) of v are not well handled and the
> TARGET_MIN_VLEN is 0 which is not reinitialized. Then the
> size / TARGET_MIN_VLEN will have DivideByZero.
> 2. The machine modes of the vector types will be vary after
> the v extension is introduced.
>
> This patch passed below testsuite:
> 1. The riscv fully regression test.
>
> PR target/114352
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc (riscv_subset_list::parse):
> Replace implied, combine and check to func finalize.
> (riscv_subset_list::finalize): New func impl to take care of
> implied, combine ext and related checks.
> * config/riscv/riscv-subset.h: Add func decl for finalize.
> * config/riscv/riscv-target-attr.cc (riscv_target_attr_parser::parse_arch):
> Finalize the ext before return succeed.
> * config/riscv/riscv.cc (riscv_set_current_function): Reinit the
> machine mode before when set cur function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/base/pr114352-1.c: New test.
> * gcc.target/riscv/rvv/base/pr114352-2.c: New test.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
> gcc/common/config/riscv/riscv-common.cc | 31 ++++++----
> gcc/config/riscv/riscv-subset.h | 2 +
> gcc/config/riscv/riscv-target-attr.cc | 2 +
> gcc/config/riscv/riscv.cc | 4 ++
> .../gcc.target/riscv/rvv/base/pr114352-1.c | 58 +++++++++++++++++++
> .../gcc.target/riscv/rvv/base/pr114352-2.c | 27 +++++++++
> 6 files changed, 114 insertions(+), 10 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
> index 440127a2af0..15d44245b3c 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -1428,16 +1428,7 @@ riscv_subset_list::parse (const char *arch, location_t loc)
> if (p == NULL)
> goto fail;
>
> - for (itr = subset_list->m_head; itr != NULL; itr = itr->next)
> - {
> - subset_list->handle_implied_ext (itr->name.c_str ());
> - }
> -
> - /* Make sure all implied extensions are included. */
> - gcc_assert (subset_list->check_implied_ext ());
> -
> - subset_list->handle_combine_ext ();
> - subset_list->check_conflict_ext ();
> + subset_list->finalize ();
>
> return subset_list;
>
> @@ -1467,6 +1458,26 @@ riscv_subset_list::set_loc (location_t loc)
> m_loc = loc;
> }
>
> +/* Make sure the implied or combined extension is included after add
> + a new std extension to subset list or likewise. For exmaple as below,
> +
> + void __attribute__((target("arch=+v"))) func () with -march=rv64gc.
> +
> + The implied zvl128b and zve64d of the std v should be included. */
> +void
> +riscv_subset_list::finalize ()
> +{
> + riscv_subset_t *subset;
> +
> + for (subset = m_head; subset != NULL; subset = subset->next)
> + handle_implied_ext (subset->name.c_str ());
> +
> + gcc_assert (check_implied_ext ());
> +
> + handle_combine_ext ();
> + check_conflict_ext ();
> +}
> +
> /* Return the current arch string. */
>
> std::string
> diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
> index ae849e2a302..ec979040e8c 100644
> --- a/gcc/config/riscv/riscv-subset.h
> +++ b/gcc/config/riscv/riscv-subset.h
> @@ -105,6 +105,8 @@ public:
> int match_score (riscv_subset_list *) const;
>
> void set_loc (location_t);
> +
> + void finalize ();
> };
>
> extern const riscv_subset_list *riscv_current_subset_list (void);
> diff --git a/gcc/config/riscv/riscv-target-attr.cc b/gcc/config/riscv/riscv-target-attr.cc
> index 9dbb78f28cc..37645adbb20 100644
> --- a/gcc/config/riscv/riscv-target-attr.cc
> +++ b/gcc/config/riscv/riscv-target-attr.cc
> @@ -136,6 +136,8 @@ riscv_target_attr_parser::parse_arch (const char *str)
> }
> token = strtok_r (NULL, ",", &str_to_check);
> }
> +
> + m_subset_list->finalize ();
> return true;
> }
> fail:
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 02a927f96b4..742974aec33 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -9492,6 +9492,10 @@ riscv_set_current_function (tree decl)
> cl_target_option_restore (&global_options, &global_options_set,
> TREE_TARGET_OPTION (new_tree));
>
> + /* The ISA extension can vary based on the function extension like target.
> + Thus, make sure that the machine modes are reflected correctly here. */
> + init_adjust_machine_modes ();
> +
> riscv_save_restore_target_globals (new_tree);
> }
>
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> new file mode 100644
> index 00000000000..b3f1f20fb79
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> @@ -0,0 +1,58 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64 -O3 -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_1:
> +** beq\s+a3,\s*zero,\s*\.L[0-9]+
> +** ...
> +** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
> +** ...
> +** ret
> +*/
> +void
> +test_1 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/*
> +** test_2:
> +** ...
> +** vsetvli\s+[atx][0-9]+,\s*[atx][0-9]+,\s*e32,\s*m1,\s*ta,\s*ma
> +** ...
> +** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
> +** ...
> +*/
> +void
> +__attribute__((target("arch=+v")))
> +test_2 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/*
> +** test_3:
> +** beq\s+a3,\s*zero,\s*\.L[0-9]+
> +** ...
> +** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
> +** ...
> +** ret
> +*/
> +void
> +test_3 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/* { dg-final { scan-assembler ".attribute arch, \"rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0\"" } } */
> +/* { dg-final { scan-assembler ".option arch, rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicsr2p0_zifencei2p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0" } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
> new file mode 100644
> index 00000000000..3b3d69d2751
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
> @@ -0,0 +1,27 @@
> +/* Test that we do not have ice when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64 -O3" } */
> +
> +#define DEF_ATTR_FUNC(ATTR, ID) \
> +void ATTR \
> +test_##ID (int *a, int *b, int *out, unsigned count) \
> +{ \
> + unsigned i; \
> + \
> + for (i = 0; i < count; i++) \
> + out[i] = a[i] + b[i]; \
> +}
> +
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve32x"))), 1)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve32f"))), 2)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64x"))), 3)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64f"))), 4)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64d"))), 5)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+v"))), 6)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl64b"))), 7)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl128b"))), 8)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl256b"))), 9)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl512b"))), 10)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl1024b"))), 11)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl2048b"))), 12)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl4096b"))), 13)
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] RISC-V: Bugfix ICE for __attribute__((target("arch=+v"))
2024-03-22 1:12 ` [PATCH v2] " pan2.li
2024-03-22 2:24 ` Kito Cheng
@ 2024-03-30 13:20 ` Andreas Schwab
1 sibling, 0 replies; 8+ messages in thread
From: Andreas Schwab @ 2024-03-30 13:20 UTC (permalink / raw)
To: pan2.li; +Cc: gcc-patches, juzhe.zhong, kito.cheng, yanzhang.wang
../../gcc/common/config/riscv/riscv-common.cc: In static member function 'static riscv_subset_list* riscv_subset_list::parse(const char*, location_t)':
../../gcc/common/config/riscv/riscv-common.cc:1501:19: error: unused variable 'itr' [-Werror=unused-variable]
1501 | riscv_subset_t *itr;
| ^~~
cc1plus: all warnings being treated as errors
make[3]: *** [Makefile:2563: riscv-common.o] Error 1
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-03-30 13:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-18 6:05 [PATCH v1] RISC-V: Bugfix ICE for __attribute__((target("arch=+v")) pan2.li
2024-03-18 6:21 ` juzhe.zhong
2024-03-21 12:38 ` Kito Cheng
2024-03-21 14:06 ` Li, Pan2
2024-03-22 1:12 ` [PATCH v2] " pan2.li
2024-03-22 2:24 ` Kito Cheng
2024-03-22 2:38 ` Li, Pan2
2024-03-30 13:20 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).