[RFC,1/2] RISC-V: __builtin_riscv_pause for all environment
Checks
Commit Message
From: Tsukasa OI <research_trasio@irq.a4lg.com>
The "pause" RISC-V hint instruction requires the 'Zihintpause' extension
(in the assembler). However, GCC emits "pause" unconditionally, making
an assembler error while compiling code with __builtin_riscv_pause while
the 'Zihintpause' extension disabled.
However, the "pause" instruction code (0x0100000f) is a HINT and emitting
its instruction code is safe in any environment.
This commit implements handling for the 'Zihintpause' extension and emits
".insn 0x0100000f" instead of "pause" only if the extension is disabled
(making the diagnostics better).
gcc/ChangeLog:
* common/config/riscv/riscv-common.cc
(riscv_ext_version_table): Implement the 'Zihintpause' extension,
version 2.0. (riscv_ext_flag_table) Add 'Zihintpause' handling.
* config/riscv/riscv-builtins.cc: Remove availability predicate
"always" and add "hint_pause" and "hint_pause_pseudo", corresponding
the existence of the 'Zihintpause' extension.
(riscv_builtins) Split builtin implementation depending on the
existence of the 'Zihintpause' extension.
* config/riscv/riscv-opts.h
(MASK_ZIHINTPAUSE, TARGET_ZIHINTPAUSE): New.
* config/riscv/riscv.md (riscv_pause): Make it only available when
the 'Zihintpause' extension is enabled. (riscv_pause_insn) New
"pause" implementation when the 'Zihintpause' extension is disabled.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/builtin_pause.c: Removed.
* gcc.target/riscv/zihintpause.c:
New test when the 'Zihintpause' extension is enabled.
* gcc.target/riscv/zihintpause-noarch.c:
New test when the 'Zihintpause' extension is disabled.
---
gcc/common/config/riscv/riscv-common.cc | 2 ++
gcc/config/riscv/riscv-builtins.cc | 6 ++++--
gcc/config/riscv/riscv-opts.h | 2 ++
gcc/config/riscv/riscv.md | 7 ++++++-
gcc/testsuite/gcc.target/riscv/builtin_pause.c | 10 ----------
gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c | 11 +++++++++++
gcc/testsuite/gcc.target/riscv/zihintpause.c | 11 +++++++++++
7 files changed, 36 insertions(+), 13 deletions(-)
delete mode 100644 gcc/testsuite/gcc.target/riscv/builtin_pause.c
create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c
create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause.c
Comments
On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote:
> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>
> The "pause" RISC-V hint instruction requires the 'Zihintpause' extension
> (in the assembler). However, GCC emits "pause" unconditionally, making
> an assembler error while compiling code with __builtin_riscv_pause while
> the 'Zihintpause' extension disabled.
>
> However, the "pause" instruction code (0x0100000f) is a HINT and emitting
> its instruction code is safe in any environment.
>
> This commit implements handling for the 'Zihintpause' extension and emits
> ".insn 0x0100000f" instead of "pause" only if the extension is disabled
> (making the diagnostics better).
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc
> (riscv_ext_version_table): Implement the 'Zihintpause' extension,
> version 2.0. (riscv_ext_flag_table) Add 'Zihintpause' handling.
> * config/riscv/riscv-builtins.cc: Remove availability predicate
> "always" and add "hint_pause" and "hint_pause_pseudo", corresponding
> the existence of the 'Zihintpause' extension.
> (riscv_builtins) Split builtin implementation depending on the
> existence of the 'Zihintpause' extension.
> * config/riscv/riscv-opts.h
> (MASK_ZIHINTPAUSE, TARGET_ZIHINTPAUSE): New.
> * config/riscv/riscv.md (riscv_pause): Make it only available when
> the 'Zihintpause' extension is enabled. (riscv_pause_insn) New
> "pause" implementation when the 'Zihintpause' extension is disabled.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/builtin_pause.c: Removed.
> * gcc.target/riscv/zihintpause.c:
> New test when the 'Zihintpause' extension is enabled.
> * gcc.target/riscv/zihintpause-noarch.c:
> New test when the 'Zihintpause' extension is disabled.
So I cleaned this up a bit per the list discussion and pushed the final
result. Hopefully I didn't goof anything too badly ;-) The net is we
emit "pause" or a suitable .insn based on TARGET_ZIHINTPAUSE.
Jeff
commit c2d04dd659c499d8df19f68d0602ad4c7d7065c2
Author: Tsukasa OI <research_trasio@irq.a4lg.com>
Date: Mon Aug 28 15:04:13 2023 -0600
RISC-V: __builtin_riscv_pause for all environment
The "pause" RISC-V hint instruction requires the 'Zihintpause' extension (in
the assembler). However, GCC emits "pause" unconditionally, making an
assembler error while compiling code with __builtin_riscv_pause while the
'Zihintpause' extension disabled.
However, the "pause" instruction code (0x0100000f) is a HINT and emitting its
instruction code is safe in any environment.
This commit implements handling for the 'Zihintpause' extension and emits
".insn 0x0100000f" instead of "pause" only if the extension is disabled (making
the diagnostics better).
gcc/ChangeLog:
* common/config/riscv/riscv-common.cc (riscv_ext_version_table):
Implement the 'Zihintpause' extension, version 2.0.
(riscv_ext_flag_table) Add 'Zihintpause' handling.
* config/riscv/riscv-builtins.cc: Remove availability predicate
"always" and add "hint_pause".
(riscv_builtins) : Add "pause" extension.
* config/riscv/riscv-opts.h (MASK_ZIHINTPAUSE, TARGET_ZIHINTPAUSE): New.
* config/riscv/riscv.md (riscv_pause): Adjust output based on
TARGET_ZIHINTPAUSE.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/builtin_pause.c: Removed.
* gcc.target/riscv/zihintpause-1.c: New test when the 'Zihintpause'
extension is enabled.
* gcc.target/riscv/zihintpause-2.c: Likewise.
* gcc.target/riscv/zihintpause-noarch.c: New test when the 'Zihintpause'
extension is disabled.
diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
index 128a7020172..a5b62cda3a0 100644
--- a/gcc/common/config/riscv/riscv-common.cc
+++ b/gcc/common/config/riscv/riscv-common.cc
@@ -224,6 +224,7 @@ static const struct riscv_ext_version riscv_ext_version_table[] =
{"zkt", ISA_SPEC_CLASS_NONE, 1, 0},
{"zihintntl", ISA_SPEC_CLASS_NONE, 1, 0},
+ {"zihintpause", ISA_SPEC_CLASS_NONE, 2, 0},
{"zicboz",ISA_SPEC_CLASS_NONE, 1, 0},
{"zicbom",ISA_SPEC_CLASS_NONE, 1, 0},
@@ -1381,6 +1382,7 @@ static const riscv_ext_flag_table_t riscv_ext_flag_table[] =
{"zkt", &gcc_options::x_riscv_zk_subext, MASK_ZKT},
{"zihintntl", &gcc_options::x_riscv_zi_subext, MASK_ZIHINTNTL},
+ {"zihintpause", &gcc_options::x_riscv_zi_subext, MASK_ZIHINTPAUSE},
{"zicboz", &gcc_options::x_riscv_zicmo_subext, MASK_ZICBOZ},
{"zicbom", &gcc_options::x_riscv_zicmo_subext, MASK_ZICBOM},
diff --git a/gcc/config/riscv/riscv-builtins.cc b/gcc/config/riscv/riscv-builtins.cc
index 79681d75962..8afe7b7e97d 100644
--- a/gcc/config/riscv/riscv-builtins.cc
+++ b/gcc/config/riscv/riscv-builtins.cc
@@ -122,7 +122,7 @@ AVAIL (clmul_zbkc32_or_zbc32, (TARGET_ZBKC || TARGET_ZBC) && !TARGET_64BIT)
AVAIL (clmul_zbkc64_or_zbc64, (TARGET_ZBKC || TARGET_ZBC) && TARGET_64BIT)
AVAIL (clmulr_zbc32, TARGET_ZBC && !TARGET_64BIT)
AVAIL (clmulr_zbc64, TARGET_ZBC && TARGET_64BIT)
-AVAIL (always, (!0))
+AVAIL (hint_pause, (!0))
/* Construct a riscv_builtin_description from the given arguments.
@@ -179,7 +179,7 @@ static const struct riscv_builtin_description riscv_builtins[] = {
DIRECT_BUILTIN (frflags, RISCV_USI_FTYPE, hard_float),
DIRECT_NO_TARGET_BUILTIN (fsflags, RISCV_VOID_FTYPE_USI, hard_float),
- DIRECT_NO_TARGET_BUILTIN (pause, RISCV_VOID_FTYPE, always),
+ RISCV_BUILTIN (pause, "pause", RISCV_BUILTIN_DIRECT_NO_TARGET, RISCV_VOID_FTYPE, hint_pause),
};
/* Index I is the function declaration for riscv_builtins[I], or null if the
diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
index b1e05967c1f..5ed69abd214 100644
--- a/gcc/config/riscv/riscv-opts.h
+++ b/gcc/config/riscv/riscv-opts.h
@@ -102,10 +102,12 @@ enum riscv_entity
#define MASK_ZICSR (1 << 0)
#define MASK_ZIFENCEI (1 << 1)
#define MASK_ZIHINTNTL (1 << 2)
+#define MASK_ZIHINTPAUSE (1 << 3)
#define TARGET_ZICSR ((riscv_zi_subext & MASK_ZICSR) != 0)
#define TARGET_ZIFENCEI ((riscv_zi_subext & MASK_ZIFENCEI) != 0)
#define TARGET_ZIHINTNTL ((riscv_zi_subext & MASK_ZIHINTNTL) != 0)
+#define TARGET_ZIHINTPAUSE ((riscv_zi_subext & MASK_ZIHINTPAUSE) != 0)
#define MASK_ZAWRS (1 << 0)
#define TARGET_ZAWRS ((riscv_za_subext & MASK_ZAWRS) != 0)
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 47d14d99903..87f4a4a33f9 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2275,7 +2275,7 @@ (define_insn "fence_i"
(define_insn "riscv_pause"
[(unspec_volatile [(const_int 0)] UNSPECV_PAUSE)]
""
- "pause")
+ "* return TARGET_ZIHINTPAUSE ? \"pause\" : \".insn\t0x0100000f\";")
;;
;; ....................
diff --git a/gcc/testsuite/gcc.target/riscv/builtin_pause.c b/gcc/testsuite/gcc.target/riscv/builtin_pause.c
deleted file mode 100644
index 9250937cabb..00000000000
--- a/gcc/testsuite/gcc.target/riscv/builtin_pause.c
+++ /dev/null
@@ -1,10 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2" } */
-
-void test_pause()
-{
- __builtin_riscv_pause ();
-}
-
-/* { dg-final { scan-assembler "pause" } } */
-
diff --git a/gcc/testsuite/gcc.target/riscv/zihintpause-1.c b/gcc/testsuite/gcc.target/riscv/zihintpause-1.c
new file mode 100644
index 00000000000..fc86efe5590
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zihintpause-1.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64i_zihintpause -mabi=lp64" } */
+/* { dg-skip-if "" { *-*-* } { "-g" "-flto"} } */
+
+void
+test ()
+{
+ __builtin_riscv_pause ();
+}
+
+/* { dg-final { scan-assembler-times "\tpause" 1 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/zihintpause-2.c b/gcc/testsuite/gcc.target/riscv/zihintpause-2.c
new file mode 100644
index 00000000000..4eaca95e9f0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zihintpause-2.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv32i_zihintpause -mabi=ilp32" } */
+/* { dg-skip-if "" { *-*-* } { "-g" "-flto"} } */
+
+void
+test ()
+{
+ __builtin_riscv_pause ();
+}
+
+/* { dg-final { scan-assembler-times "\tpause" 1 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c b/gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c
new file mode 100644
index 00000000000..7ce5cba90d5
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64i -mabi=lp64" { target { rv64 } } } */
+/* { dg-options "-march=rv32i -mabi=ilp32" { target { rv32 } } } */
+/* { dg-skip-if "" { *-*-* } { "-g" "-flto"} } */
+
+void
+test ()
+{
+ __builtin_riscv_pause ();
+}
+
+/* { dg-final { scan-assembler-times "0x0100000f" 1 } } */
On 2023/08/29 6:12, Jeff Law wrote:
>
>
> On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote:
>> From: Tsukasa OI <research_trasio@irq.a4lg.com>
>>
>> The "pause" RISC-V hint instruction requires the 'Zihintpause' extension
>> (in the assembler). However, GCC emits "pause" unconditionally, making
>> an assembler error while compiling code with __builtin_riscv_pause while
>> the 'Zihintpause' extension disabled.
>>
>> However, the "pause" instruction code (0x0100000f) is a HINT and emitting
>> its instruction code is safe in any environment.
>>
>> This commit implements handling for the 'Zihintpause' extension and emits
>> ".insn 0x0100000f" instead of "pause" only if the extension is disabled
>> (making the diagnostics better).
>>
>> gcc/ChangeLog:
>>
>> * common/config/riscv/riscv-common.cc
>> (riscv_ext_version_table): Implement the 'Zihintpause' extension,
>> version 2.0. (riscv_ext_flag_table) Add 'Zihintpause' handling.
>> * config/riscv/riscv-builtins.cc: Remove availability predicate
>> "always" and add "hint_pause" and "hint_pause_pseudo", corresponding
>> the existence of the 'Zihintpause' extension.
>> (riscv_builtins) Split builtin implementation depending on the
>> existence of the 'Zihintpause' extension.
>> * config/riscv/riscv-opts.h
>> (MASK_ZIHINTPAUSE, TARGET_ZIHINTPAUSE): New.
>> * config/riscv/riscv.md (riscv_pause): Make it only available when
>> the 'Zihintpause' extension is enabled. (riscv_pause_insn) New
>> "pause" implementation when the 'Zihintpause' extension is disabled.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/builtin_pause.c: Removed.
>> * gcc.target/riscv/zihintpause.c:
>> New test when the 'Zihintpause' extension is enabled.
>> * gcc.target/riscv/zihintpause-noarch.c:
>> New test when the 'Zihintpause' extension is disabled.
> So I cleaned this up a bit per the list discussion and pushed the final
> result. Hopefully I didn't goof anything too badly ;-) The net is we
> emit "pause" or a suitable .insn based on TARGET_ZIHINTPAUSE.
>
> Jeff
Thanks! I had having a problem to type words through the keyboard for a
while and I appreciate doing that instead of me (your modifications were
mostly "I would do so too" ones).
Also, it seems that I will no longer need to ask you to remove leading
"[PATCH xxx]" (not just the commit title is not my intention, I worried
that you have been doing something inefficient [other than "git am"]).
Tsukasa
@@ -209,6 +209,7 @@ static const struct riscv_ext_version riscv_ext_version_table[] =
{"zkt", ISA_SPEC_CLASS_NONE, 1, 0},
{"zihintntl", ISA_SPEC_CLASS_NONE, 1, 0},
+ {"zihintpause", ISA_SPEC_CLASS_NONE, 2, 0},
{"zicboz",ISA_SPEC_CLASS_NONE, 1, 0},
{"zicbom",ISA_SPEC_CLASS_NONE, 1, 0},
@@ -1344,6 +1345,7 @@ static const riscv_ext_flag_table_t riscv_ext_flag_table[] =
{"zkt", &gcc_options::x_riscv_zk_subext, MASK_ZKT},
{"zihintntl", &gcc_options::x_riscv_zi_subext, MASK_ZIHINTNTL},
+ {"zihintpause", &gcc_options::x_riscv_zi_subext, MASK_ZIHINTPAUSE},
{"zicboz", &gcc_options::x_riscv_zicmo_subext, MASK_ZICBOZ},
{"zicbom", &gcc_options::x_riscv_zicmo_subext, MASK_ZICBOM},
@@ -122,7 +122,8 @@ AVAIL (clmul_zbkc32_or_zbc32, (TARGET_ZBKC || TARGET_ZBC) && !TARGET_64BIT)
AVAIL (clmul_zbkc64_or_zbc64, (TARGET_ZBKC || TARGET_ZBC) && TARGET_64BIT)
AVAIL (clmulr_zbc32, TARGET_ZBC && !TARGET_64BIT)
AVAIL (clmulr_zbc64, TARGET_ZBC && TARGET_64BIT)
-AVAIL (always, (!0))
+AVAIL (hint_pause, TARGET_ZIHINTPAUSE)
+AVAIL (hint_pause_pseudo, !TARGET_ZIHINTPAUSE)
/* Construct a riscv_builtin_description from the given arguments.
@@ -179,7 +180,8 @@ static const struct riscv_builtin_description riscv_builtins[] = {
DIRECT_BUILTIN (frflags, RISCV_USI_FTYPE, hard_float),
DIRECT_NO_TARGET_BUILTIN (fsflags, RISCV_VOID_FTYPE_USI, hard_float),
- DIRECT_NO_TARGET_BUILTIN (pause, RISCV_VOID_FTYPE, always),
+ RISCV_BUILTIN (pause, "pause", RISCV_BUILTIN_DIRECT_NO_TARGET, RISCV_VOID_FTYPE, hint_pause),
+ RISCV_BUILTIN (pause_insn, "pause", RISCV_BUILTIN_DIRECT_NO_TARGET, RISCV_VOID_FTYPE, hint_pause_pseudo),
};
/* Index I is the function declaration for riscv_builtins[I], or null if the
@@ -102,10 +102,12 @@ enum riscv_entity
#define MASK_ZICSR (1 << 0)
#define MASK_ZIFENCEI (1 << 1)
#define MASK_ZIHINTNTL (1 << 2)
+#define MASK_ZIHINTPAUSE (1 << 3)
#define TARGET_ZICSR ((riscv_zi_subext & MASK_ZICSR) != 0)
#define TARGET_ZIFENCEI ((riscv_zi_subext & MASK_ZIFENCEI) != 0)
#define TARGET_ZIHINTNTL ((riscv_zi_subext & MASK_ZIHINTNTL) != 0)
+#define TARGET_ZIHINTPAUSE ((riscv_zi_subext & MASK_ZIHINTPAUSE) != 0)
#define MASK_ZAWRS (1 << 0)
#define TARGET_ZAWRS ((riscv_za_subext & MASK_ZAWRS) != 0)
@@ -2192,9 +2192,14 @@
(define_insn "riscv_pause"
[(unspec_volatile [(const_int 0)] UNSPECV_PAUSE)]
- ""
+ "TARGET_ZIHINTPAUSE"
"pause")
+(define_insn "riscv_pause_insn"
+ [(unspec_volatile [(const_int 0)] UNSPECV_PAUSE)]
+ ""
+ ".insn 0x0100000f")
+
;;
;; ....................
;;
deleted file mode 100644
@@ -1,10 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2" } */
-
-void test_pause()
-{
- __builtin_riscv_pause ();
-}
-
-/* { dg-final { scan-assembler "pause" } } */
-
new file mode 100644
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64i -mabi=lp64" { target { rv64 } } } */
+/* { dg-options "-march=rv32i -mabi=ilp32" { target { rv32 } } } */
+/* { dg-skip-if "" { *-*-* } { "-g" "-flto"} } */
+
+void test_pause()
+{
+ __builtin_riscv_pause ();
+}
+
+/* { dg-final { scan-assembler-times "0x0100000f" 1 } } */
new file mode 100644
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64i_zihintpause -mabi=lp64" { target { rv64 } } } */
+/* { dg-options "-march=rv32i_zihintpause -mabi=ilp32" { target { rv32 } } } */
+/* { dg-skip-if "" { *-*-* } { "-g" "-flto"} } */
+
+void test_pause()
+{
+ __builtin_riscv_pause ();
+}
+
+/* { dg-final { scan-assembler-times "\tpause" 1 } } */