* [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment @ 2023-08-09 6:11 Tsukasa OI 2023-08-09 6:11 ` [RFC PATCH 1/2] " Tsukasa OI ` (4 more replies) 0 siblings, 5 replies; 28+ messages in thread From: Tsukasa OI @ 2023-08-09 6:11 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches Hello, I found that a built-in function "__builtin_riscv_pause" is not usable unless we enable the 'Zihintpause' extension explicitly (still, this built-in exists EVEN IF the 'Zihintpause' extension is disabled). Contents of a.c: > void sample(void) > { > __builtin_riscv_pause(); > } Compiling with the 'Zihintpause' extension is fine. > $ riscv64-unknown-elf-gcc -O2 -march=rv64i_zihintpause -mabi=lp64 -c a.c However, compiling without the 'Zihintpause' causes an assembler error (tested with GNU Binutils 2.41): > $ riscv64-unknown-elf-gcc -O2 -march=rv64i -mabi=lp64 -c a.c > /tmp/ccFjacAz.s: Assembler messages: > /tmp/ccFjacAz.s:11: Error: unrecognized opcode `pause', extension `zihintpause' required This is because: 1. GCC does not handle the 'Zihintpause' extension and 2. "riscv_pause" (insn) unconditionally emits "pause" even if the assembler does not accept it (when the extension is disabled). This patch set (PATCH 1/2) resolves this issue by: 1. Handling the 'Zihintpause' extension and 2. Splitting the "__builtin_riscv_pause" implementation depending on the existence of the 'Zihintpause' extension. Because a released version of GCC defines "__builtin_riscv_pause" unconditionally, I chose to define no-'Zihintpause' version. There is another option to unconditionally emit ".insn 0x0100000f" (the machine code of "pause") but I didn't because I wanted to improve the diagnostics (e.g. *.s output). I also fixed the description of this built-in function (in PATCH 2/2). I'm not sure whether this is a good method to split the implementation depending on the 'Zihintpause' extension. Other than that, I believe that this is okay and approval is appreciated. Note that because I assigned copyright of GCC contribution to FSF, I didn't attach "Signed-off-by" tag. Tell me if I need it. Thanks, Tsukasa Tsukasa OI (2): RISC-V: __builtin_riscv_pause for all environment RISC-V: Fix documentation of __builtin_riscv_pause 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/doc/extend.texi | 6 +++--- 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 +++++++++++ 8 files changed, 39 insertions(+), 16 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 base-commit: c8b396243ec5bfa9b541555131df597ebc84b9d0 -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH 1/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-09 6:11 [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Tsukasa OI @ 2023-08-09 6:11 ` Tsukasa OI 2023-08-28 21:12 ` Jeff Law 2023-08-09 6:11 ` [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI ` (3 subsequent siblings) 4 siblings, 1 reply; 28+ messages in thread From: Tsukasa OI @ 2023-08-09 6:11 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches 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 diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc index 2eb8c7cadff0..02502ba07e82 100644 --- a/gcc/common/config/riscv/riscv-common.cc +++ b/gcc/common/config/riscv/riscv-common.cc @@ -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}, diff --git a/gcc/config/riscv/riscv-builtins.cc b/gcc/config/riscv/riscv-builtins.cc index 79681d759628..554fb7f69bb0 100644 --- a/gcc/config/riscv/riscv-builtins.cc +++ b/gcc/config/riscv/riscv-builtins.cc @@ -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 diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h index 28d9b81bd800..a6c3e0c9098f 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 688fd697255b..f2fcbfa6163b 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -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") + ;; ;; .................... ;; 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 9250937cabb9..000000000000 --- 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-noarch.c b/gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c new file mode 100644 index 000000000000..99ab953bd1db --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c @@ -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 } } */ diff --git a/gcc/testsuite/gcc.target/riscv/zihintpause.c b/gcc/testsuite/gcc.target/riscv/zihintpause.c new file mode 100644 index 000000000000..575b42277705 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/zihintpause.c @@ -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 } } */ -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-09 6:11 ` [RFC PATCH 1/2] " Tsukasa OI @ 2023-08-28 21:12 ` Jeff Law 2023-08-29 2:02 ` Tsukasa OI 0 siblings, 1 reply; 28+ messages in thread From: Jeff Law @ 2023-08-28 21:12 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 1965 bytes --] 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 [-- Attachment #2: P --] [-- Type: text/plain, Size: 6914 bytes --] 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 } } */ ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 1/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-28 21:12 ` Jeff Law @ 2023-08-29 2:02 ` Tsukasa OI 0 siblings, 0 replies; 28+ messages in thread From: Tsukasa OI @ 2023-08-29 2:02 UTC (permalink / raw) To: Jeff Law; +Cc: GCC Patches 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 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause 2023-08-09 6:11 [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Tsukasa OI 2023-08-09 6:11 ` [RFC PATCH 1/2] " Tsukasa OI @ 2023-08-09 6:11 ` Tsukasa OI 2023-08-28 21:14 ` Jeff Law 2023-08-09 20:05 ` [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Jeff Law ` (2 subsequent siblings) 4 siblings, 1 reply; 28+ messages in thread From: Tsukasa OI @ 2023-08-09 6:11 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches From: Tsukasa OI <research_trasio@irq.a4lg.com> This built-in does not imply the 'Xgnuzihintpausestate' extension. It does not change architectural state (because all HINTs are prohibited from doing that). gcc/ChangeLog: * doc/extend.texi: Fix the description of __builtin_riscv_pause. --- gcc/doc/extend.texi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 89c5b4ea2b20..7ebbe70c78d6 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -21570,9 +21570,9 @@ Returns the value that is currently set in the @samp{tp} register. @enddefbuiltin @defbuiltin{void __builtin_riscv_pause (void)} -Generates the @code{pause} (hint) machine instruction. This implies the -Xgnuzihintpausestate extension, which redefines the @code{pause} instruction to -change architectural state. +Generates the @code{pause} (hint) machine instruction. If the target implements +the Zihintpause extension, it indicates that the current hart should be +temporarily paused or slowed down. @enddefbuiltin @node RISC-V Vector Intrinsics -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause 2023-08-09 6:11 ` [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI @ 2023-08-28 21:14 ` Jeff Law 2023-08-28 23:09 ` Hans-Peter Nilsson 0 siblings, 1 reply; 28+ messages in thread From: Jeff Law @ 2023-08-28 21:14 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote: > From: Tsukasa OI <research_trasio@irq.a4lg.com> > > This built-in does not imply the 'Xgnuzihintpausestate' extension. > It does not change architectural state (because all HINTs are prohibited > from doing that). > > gcc/ChangeLog: > > * doc/extend.texi: Fix the description of __builtin_riscv_pause. I've pushed this to the trunk. jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause 2023-08-28 21:14 ` Jeff Law @ 2023-08-28 23:09 ` Hans-Peter Nilsson 2023-08-28 23:15 ` Jeff Law 2023-08-29 2:15 ` Tsukasa OI 0 siblings, 2 replies; 28+ messages in thread From: Hans-Peter Nilsson @ 2023-08-28 23:09 UTC (permalink / raw) To: Jeff Law; +Cc: Tsukasa OI, gcc-patches On Mon, 28 Aug 2023, Jeff Law via Gcc-patches wrote: > > > On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote: > > From: Tsukasa OI <research_trasio@irq.a4lg.com> > > > > This built-in does not imply the 'Xgnuzihintpausestate' extension. > > It does not change architectural state (because all HINTs are prohibited > > from doing that). > > > > gcc/ChangeLog: > > > > * doc/extend.texi: Fix the description of __builtin_riscv_pause. > I've pushed this to the trunk. I randomly noticed a typo: "hart", perhaps for "part"? Not sure though. brgds, H-P ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause 2023-08-28 23:09 ` Hans-Peter Nilsson @ 2023-08-28 23:15 ` Jeff Law 2023-08-29 2:15 ` Tsukasa OI 1 sibling, 0 replies; 28+ messages in thread From: Jeff Law @ 2023-08-28 23:15 UTC (permalink / raw) To: Hans-Peter Nilsson; +Cc: Tsukasa OI, gcc-patches On 8/28/23 17:09, Hans-Peter Nilsson wrote: > On Mon, 28 Aug 2023, Jeff Law via Gcc-patches wrote: >> >> >> On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote: >>> From: Tsukasa OI <research_trasio@irq.a4lg.com> >>> >>> This built-in does not imply the 'Xgnuzihintpausestate' extension. >>> It does not change architectural state (because all HINTs are prohibited >>> from doing that). >>> >>> gcc/ChangeLog: >>> >>> * doc/extend.texi: Fix the description of __builtin_riscv_pause. >> I've pushed this to the trunk. > > I randomly noticed a typo: "hart", perhaps for "part"? > Not sure though. Not a typo. "hart" has a well defined meaning in the risc-v world. Thanks, jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause 2023-08-28 23:09 ` Hans-Peter Nilsson 2023-08-28 23:15 ` Jeff Law @ 2023-08-29 2:15 ` Tsukasa OI 2023-08-29 9:26 ` Hans-Peter Nilsson 1 sibling, 1 reply; 28+ messages in thread From: Tsukasa OI @ 2023-08-29 2:15 UTC (permalink / raw) To: Hans-Peter Nilsson, Jeff Law; +Cc: gcc-patches On 2023/08/29 8:09, Hans-Peter Nilsson wrote: > On Mon, 28 Aug 2023, Jeff Law via Gcc-patches wrote: >> >> >> On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote: >>> From: Tsukasa OI <research_trasio@irq.a4lg.com> >>> >>> This built-in does not imply the 'Xgnuzihintpausestate' extension. >>> It does not change architectural state (because all HINTs are prohibited >>> from doing that). >>> >>> gcc/ChangeLog: >>> >>> * doc/extend.texi: Fix the description of __builtin_riscv_pause. >> I've pushed this to the trunk. > > I randomly noticed a typo: "hart", perhaps for "part"? > Not sure though. > > brgds, H-P > Hi H-P, As Jeff mentioned you, the word "hart" in the RISC-V world means a HARdware Thread and commonly used to represent a hardware-based unit of execution. Tsukasa ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause 2023-08-29 2:15 ` Tsukasa OI @ 2023-08-29 9:26 ` Hans-Peter Nilsson 0 siblings, 0 replies; 28+ messages in thread From: Hans-Peter Nilsson @ 2023-08-29 9:26 UTC (permalink / raw) To: Tsukasa OI; +Cc: Jeff Law, gcc-patches On Tue, 29 Aug 2023, Tsukasa OI wrote: > On 2023/08/29 8:09, Hans-Peter Nilsson wrote: > > On Mon, 28 Aug 2023, Jeff Law via Gcc-patches wrote: > >> > >> > >> On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote: > >>> From: Tsukasa OI <research_trasio@irq.a4lg.com> > >>> > >>> This built-in does not imply the 'Xgnuzihintpausestate' extension. > >>> It does not change architectural state (because all HINTs are prohibited > >>> from doing that). > >>> > >>> gcc/ChangeLog: > >>> > >>> * doc/extend.texi: Fix the description of __builtin_riscv_pause. > >> I've pushed this to the trunk. > > > > I randomly noticed a typo: "hart", perhaps for "part"? > > Not sure though. > > > > brgds, H-P > > > > Hi H-P, > > As Jeff mentioned you, the word "hart" in the RISC-V world means a > HARdware Thread and commonly used to represent a hardware-based unit of > execution. Thanks for the explanation. Perhaps it's worth telling general readers of the document that it's a term, by simply decorating "hart" like @samp{hart} or adding the text "(hardware thread)"? It's the only grep-hit in the document at this time. brgds, H-P ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-09 6:11 [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Tsukasa OI 2023-08-09 6:11 ` [RFC PATCH 1/2] " Tsukasa OI 2023-08-09 6:11 ` [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI @ 2023-08-09 20:05 ` Jeff Law 2023-08-09 22:39 ` Tsukasa OI 2023-08-10 2:25 ` [RFC PATCH v2 " Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 0/2] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only Tsukasa OI 4 siblings, 1 reply; 28+ messages in thread From: Jeff Law @ 2023-08-09 20:05 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote: > Hello, > > I found that a built-in function "__builtin_riscv_pause" is not usable > unless we enable the 'Zihintpause' extension explicitly (still, this > built-in exists EVEN IF the 'Zihintpause' extension is disabled). > > Contents of a.c: > >> void sample(void) >> { >> __builtin_riscv_pause(); >> } > > > Compiling with the 'Zihintpause' extension is fine. > >> $ riscv64-unknown-elf-gcc -O2 -march=rv64i_zihintpause -mabi=lp64 -c a.c > > > However, compiling without the 'Zihintpause' causes an assembler error > (tested with GNU Binutils 2.41): > >> $ riscv64-unknown-elf-gcc -O2 -march=rv64i -mabi=lp64 -c a.c >> /tmp/ccFjacAz.s: Assembler messages: >> /tmp/ccFjacAz.s:11: Error: unrecognized opcode `pause', extension `zihintpause' required > > > This is because: > > 1. GCC does not handle the 'Zihintpause' extension and > 2. "riscv_pause" (insn) unconditionally emits "pause" even if the > assembler does not accept it (when the extension is disabled). > > > This patch set (PATCH 1/2) resolves this issue by: > > 1. Handling the 'Zihintpause' extension and > 2. Splitting the "__builtin_riscv_pause" implementation > depending on the existence of the 'Zihintpause' extension. > > Because a released version of GCC defines "__builtin_riscv_pause" > unconditionally, I chose to define no-'Zihintpause' version. > > There is another option to unconditionally emit ".insn 0x0100000f" > (the machine code of "pause") but I didn't because I wanted to improve the > diagnostics (e.g. *.s output). > > I also fixed the description of this built-in function (in PATCH 2/2). > > > I'm not sure whether this is a good method to split the implementation > depending on the 'Zihintpause' extension. Other than that, I believe that > this is okay and approval is appreciated. > > Note that because I assigned copyright of GCC contribution to FSF, I didn't > attach "Signed-off-by" tag. Tell me if I need it. I'd tend to think we do not want to expose the intrinsic unless the right extensions are enabled -- even though the encoding is a no-op and we could emit it as a .insn. If others think otherwise, I'll go with the consensus opinion. So let's hold off a bit until others have chimed in. Thanks, jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-09 20:05 ` [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Jeff Law @ 2023-08-09 22:39 ` Tsukasa OI 2023-08-11 23:30 ` Jeff Law 0 siblings, 1 reply; 28+ messages in thread From: Tsukasa OI @ 2023-08-09 22:39 UTC (permalink / raw) To: Jeff Law, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches On 2023/08/10 5:05, Jeff Law wrote: > > > On 8/9/23 00:11, Tsukasa OI via Gcc-patches wrote: >> Hello, >> >> I found that a built-in function "__builtin_riscv_pause" is not usable >> unless we enable the 'Zihintpause' extension explicitly (still, this >> built-in exists EVEN IF the 'Zihintpause' extension is disabled). >> >> Contents of a.c: >> >>> void sample(void) >>> { >>> __builtin_riscv_pause(); >>> } >> >> >> Compiling with the 'Zihintpause' extension is fine. >> >>> $ riscv64-unknown-elf-gcc -O2 -march=rv64i_zihintpause -mabi=lp64 -c a.c >> >> >> However, compiling without the 'Zihintpause' causes an assembler error >> (tested with GNU Binutils 2.41): >> >>> $ riscv64-unknown-elf-gcc -O2 -march=rv64i -mabi=lp64 -c a.c >>> /tmp/ccFjacAz.s: Assembler messages: >>> /tmp/ccFjacAz.s:11: Error: unrecognized opcode `pause', extension >>> `zihintpause' required >> >> >> This is because: >> >> 1. GCC does not handle the 'Zihintpause' extension and >> 2. "riscv_pause" (insn) unconditionally emits "pause" even if the >> assembler does not accept it (when the extension is disabled). >> >> >> This patch set (PATCH 1/2) resolves this issue by: >> >> 1. Handling the 'Zihintpause' extension and >> 2. Splitting the "__builtin_riscv_pause" implementation >> depending on the existence of the 'Zihintpause' extension. >> >> Because a released version of GCC defines "__builtin_riscv_pause" >> unconditionally, I chose to define no-'Zihintpause' version. >> >> There is another option to unconditionally emit ".insn 0x0100000f" >> (the machine code of "pause") but I didn't because I wanted to improve >> the >> diagnostics (e.g. *.s output). >> >> I also fixed the description of this built-in function (in PATCH 2/2). >> >> >> I'm not sure whether this is a good method to split the implementation >> depending on the 'Zihintpause' extension. Other than that, I believe >> that >> this is okay and approval is appreciated. >> >> Note that because I assigned copyright of GCC contribution to FSF, I >> didn't >> attach "Signed-off-by" tag. Tell me if I need it. > I'd tend to think we do not want to expose the intrinsic unless the > right extensions are enabled -- even though the encoding is a no-op and > we could emit it as a .insn. I think that makes sense. The only reason I implemented the no-'Zihintpause' version is because GCC 13 implemented the built-in unconditionally. If the compatibility breakage is considered minimum (I don't know, though), I'm ready to submit 'Zihintpause'-only version of this patch set. Thanks, Tsukasa > > If others think otherwise, I'll go with the consensus opinion. So let's > hold off a bit until others have chimed in. > > Thanks, > jeff > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-09 22:39 ` Tsukasa OI @ 2023-08-11 23:30 ` Jeff Law 2023-08-11 23:34 ` Palmer Dabbelt ` (2 more replies) 0 siblings, 3 replies; 28+ messages in thread From: Jeff Law @ 2023-08-11 23:30 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches On 8/9/23 16:39, Tsukasa OI wrote: > On 2023/08/10 5:05, Jeff Law wrote: >> I'd tend to think we do not want to expose the intrinsic unless the >> right extensions are enabled -- even though the encoding is a no-op and >> we could emit it as a .insn. > > I think that makes sense. The only reason I implemented the > no-'Zihintpause' version is because GCC 13 implemented the built-in > unconditionally. If the compatibility breakage is considered minimum (I > don't know, though), I'm ready to submit 'Zihintpause'-only version of > this patch set. While it's a compatibility break I don't think we have a need to preserve this kind of compatibility. I suspect anyone using __builtin_riscv_pause was probably already turning on Zihintpause and if they weren't they should have been :-0 I'm sure we'll kick this around in the Tuesday meeting and hopefully make a decision about the desired direction. You're obviously welcome to join if you're inclined. Let me know if you need an invite. jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-11 23:30 ` Jeff Law @ 2023-08-11 23:34 ` Palmer Dabbelt 2023-08-12 0:20 ` Tsukasa OI 2023-08-13 19:52 ` Philipp Tomsich 2 siblings, 0 replies; 28+ messages in thread From: Palmer Dabbelt @ 2023-08-11 23:34 UTC (permalink / raw) To: gcc-patches Cc: research_trasio, Kito Cheng, Andrew Waterman, Jim Wilson, gcc-patches On Fri, 11 Aug 2023 16:30:29 PDT (-0700), gcc-patches@gcc.gnu.org wrote: > > > On 8/9/23 16:39, Tsukasa OI wrote: >> On 2023/08/10 5:05, Jeff Law wrote: > >>> I'd tend to think we do not want to expose the intrinsic unless the >>> right extensions are enabled -- even though the encoding is a no-op and >>> we could emit it as a .insn. >> >> I think that makes sense. The only reason I implemented the >> no-'Zihintpause' version is because GCC 13 implemented the built-in >> unconditionally. If the compatibility breakage is considered minimum (I >> don't know, though), I'm ready to submit 'Zihintpause'-only version of >> this patch set. > While it's a compatibility break I don't think we have a need to > preserve this kind of compatibility. I suspect anyone using > __builtin_riscv_pause was probably already turning on Zihintpause and if > they weren't they should have been :-0 I agree it's fine to just call this a bug: the builtin wasn't doing anything on non-Zihintpause systems anyway, so it's not like it could have been all that useful. > I'm sure we'll kick this around in the Tuesday meeting and hopefully > make a decision about the desired direction. You're obviously welcome > to join if you're inclined. Let me know if you need an invite. > > jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-11 23:30 ` Jeff Law 2023-08-11 23:34 ` Palmer Dabbelt @ 2023-08-12 0:20 ` Tsukasa OI 2023-08-15 13:49 ` Jeff Law 2023-08-13 19:52 ` Philipp Tomsich 2 siblings, 1 reply; 28+ messages in thread From: Tsukasa OI @ 2023-08-12 0:20 UTC (permalink / raw) To: Jeff Law, Palmer Dabbelt; +Cc: gcc-patches On 2023/08/12 8:30, Jeff Law wrote: > > > On 8/9/23 16:39, Tsukasa OI wrote: >> On 2023/08/10 5:05, Jeff Law wrote: > >>> I'd tend to think we do not want to expose the intrinsic unless the >>> right extensions are enabled -- even though the encoding is a no-op and >>> we could emit it as a .insn. >> >> I think that makes sense. The only reason I implemented the >> no-'Zihintpause' version is because GCC 13 implemented the built-in >> unconditionally. If the compatibility breakage is considered minimum (I >> don't know, though), I'm ready to submit 'Zihintpause'-only version of >> this patch set. > While it's a compatibility break I don't think we have a need to > preserve this kind of compatibility. I suspect anyone using > __builtin_riscv_pause was probably already turning on Zihintpause and if > they weren't they should have been :-0 > > > I'm sure we'll kick this around in the Tuesday meeting and hopefully > make a decision about the desired direction. You're obviously welcome > to join if you're inclined. Let me know if you need an invite. > > jeff > I'll not be able to attend that meeting due to Japanese religious events around Aug 13-16 (it may not be impossible but at least difficult) but look forward seeing that some conclusion is made. I leave two patch sets corresponding two options so in either case, we can apply a fix after the conclusion is made. (1) __builtin_riscv_pause for 'Zihintpause'-only <https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626912.html> (2) __builtin_riscv_pause for all <https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626909.html> Thanks, Tsukasa ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-12 0:20 ` Tsukasa OI @ 2023-08-15 13:49 ` Jeff Law 0 siblings, 0 replies; 28+ messages in thread From: Jeff Law @ 2023-08-15 13:49 UTC (permalink / raw) To: Tsukasa OI, Palmer Dabbelt; +Cc: gcc-patches On 8/11/23 18:20, Tsukasa OI wrote: >> > > I'll not be able to attend that meeting due to Japanese religious events > around Aug 13-16 (it may not be impossible but at least difficult) but > look forward seeing that some conclusion is made. No problem. We hold that meeting weekly to work through any outstanding patches related to RISC-V. You're always welcome to attend if you want. > > I leave two patch sets corresponding two options so in either case, we > can apply a fix after the conclusion is made. > > (1) __builtin_riscv_pause for 'Zihintpause'-only > <https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626912.html> > (2) __builtin_riscv_pause for all > <https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626909.html> Thanks. It's not clear what direction we'll take on this, but having a patchkit for both potential outcomes is definitely helpful. jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-11 23:30 ` Jeff Law 2023-08-11 23:34 ` Palmer Dabbelt 2023-08-12 0:20 ` Tsukasa OI @ 2023-08-13 19:52 ` Philipp Tomsich 2023-08-13 20:17 ` Andrew Waterman 2023-08-15 13:52 ` Jeff Law 2 siblings, 2 replies; 28+ messages in thread From: Philipp Tomsich @ 2023-08-13 19:52 UTC (permalink / raw) To: Jeff Law Cc: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, gcc-patches On Sat, 12 Aug 2023 at 01:31, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > On 8/9/23 16:39, Tsukasa OI wrote: > > On 2023/08/10 5:05, Jeff Law wrote: > > >> I'd tend to think we do not want to expose the intrinsic unless the > >> right extensions are enabled -- even though the encoding is a no-op and > >> we could emit it as a .insn. > > > > I think that makes sense. The only reason I implemented the > > no-'Zihintpause' version is because GCC 13 implemented the built-in > > unconditionally. If the compatibility breakage is considered minimum (I > > don't know, though), I'm ready to submit 'Zihintpause'-only version of > > this patch set. > While it's a compatibility break I don't think we have a need to > preserve this kind of compatibility. I suspect anyone using > __builtin_riscv_pause was probably already turning on Zihintpause and if > they weren't they should have been :-0 > > > I'm sure we'll kick this around in the Tuesday meeting and hopefully > make a decision about the desired direction. You're obviously welcome > to join if you're inclined. Let me know if you need an invite. The original discussion (and I believe that Andrew was the decisive voice in the end) came to the conclusion that—given that pause is a true hint—it could always be enabled. We had originally expected to enable it only if Zihintpause was part of the target architecture, but viewing it as "just a name for an already existing pure hint" also made sense. Note that on systems that don't implement Zihintpause, the hint is guarantueed to not have an architectural effect. That said, I don't really have a strong leaning one way or another. Philipp. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-13 19:52 ` Philipp Tomsich @ 2023-08-13 20:17 ` Andrew Waterman 2023-08-15 13:52 ` Jeff Law 1 sibling, 0 replies; 28+ messages in thread From: Andrew Waterman @ 2023-08-13 20:17 UTC (permalink / raw) To: Philipp Tomsich Cc: Jeff Law, Tsukasa OI, Kito Cheng, Palmer Dabbelt, Jim Wilson, gcc-patches On Sun, Aug 13, 2023 at 12:53 PM Philipp Tomsich <philipp.tomsich@vrull.eu> wrote: > > On Sat, 12 Aug 2023 at 01:31, Jeff Law via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > On 8/9/23 16:39, Tsukasa OI wrote: > > > On 2023/08/10 5:05, Jeff Law wrote: > > > > >> I'd tend to think we do not want to expose the intrinsic unless the > > >> right extensions are enabled -- even though the encoding is a no-op and > > >> we could emit it as a .insn. > > > > > > I think that makes sense. The only reason I implemented the > > > no-'Zihintpause' version is because GCC 13 implemented the built-in > > > unconditionally. If the compatibility breakage is considered minimum (I > > > don't know, though), I'm ready to submit 'Zihintpause'-only version of > > > this patch set. > > While it's a compatibility break I don't think we have a need to > > preserve this kind of compatibility. I suspect anyone using > > __builtin_riscv_pause was probably already turning on Zihintpause and if > > they weren't they should have been :-0 > > > > > > I'm sure we'll kick this around in the Tuesday meeting and hopefully > > make a decision about the desired direction. You're obviously welcome > > to join if you're inclined. Let me know if you need an invite. > > The original discussion (and I believe that Andrew was the decisive > voice in the end) came to the conclusion that—given that pause is a > true hint—it could always be enabled. I continue to think that, since it's semantically valid to execute a HINT on any implementation, there's little utility in ever rejecting the HINT builtins, or in rejecting explicit HINTs in asm, irrespective of -march. But ultimately it isn't a big deal either way. > We had originally expected to enable it only if Zihintpause was part > of the target architecture, but viewing it as "just a name for an > already existing pure hint" also made sense. > Note that on systems that don't implement Zihintpause, the hint is > guarantueed to not have an architectural effect. > > That said, I don't really have a strong leaning one way or another. > Philipp. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-13 19:52 ` Philipp Tomsich 2023-08-13 20:17 ` Andrew Waterman @ 2023-08-15 13:52 ` Jeff Law 1 sibling, 0 replies; 28+ messages in thread From: Jeff Law @ 2023-08-15 13:52 UTC (permalink / raw) To: Philipp Tomsich Cc: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, gcc-patches On 8/13/23 13:52, Philipp Tomsich wrote: > On Sat, 12 Aug 2023 at 01:31, Jeff Law via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> >> >> On 8/9/23 16:39, Tsukasa OI wrote: >>> On 2023/08/10 5:05, Jeff Law wrote: >> >>>> I'd tend to think we do not want to expose the intrinsic unless the >>>> right extensions are enabled -- even though the encoding is a no-op and >>>> we could emit it as a .insn. >>> >>> I think that makes sense. The only reason I implemented the >>> no-'Zihintpause' version is because GCC 13 implemented the built-in >>> unconditionally. If the compatibility breakage is considered minimum (I >>> don't know, though), I'm ready to submit 'Zihintpause'-only version of >>> this patch set. >> While it's a compatibility break I don't think we have a need to >> preserve this kind of compatibility. I suspect anyone using >> __builtin_riscv_pause was probably already turning on Zihintpause and if >> they weren't they should have been :-0 >> >> >> I'm sure we'll kick this around in the Tuesday meeting and hopefully >> make a decision about the desired direction. You're obviously welcome >> to join if you're inclined. Let me know if you need an invite. > > The original discussion (and I believe that Andrew was the decisive > voice in the end) came to the conclusion that—given that pause is a > true hint—it could always be enabled. > We had originally expected to enable it only if Zihintpause was part > of the target architecture, but viewing it as "just a name for an > already existing pure hint" also made sense. > Note that on systems that don't implement Zihintpause, the hint is > guarantueed to not have an architectural effect. > > That said, I don't really have a strong leaning one way or another. > Philipp. I don't have a strong opinion either way and I certainly see both sides of the argument. It sounds like the current situation is by design; knowing that now I would tend to lean towards keeping status quo, which would mean going with Tsukasa's first patch or something similar. We'll certainly discuss on the call in a half-hour or so. jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v2 0/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-09 6:11 [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Tsukasa OI ` (2 preceding siblings ...) 2023-08-09 20:05 ` [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Jeff Law @ 2023-08-10 2:25 ` Tsukasa OI 2023-08-10 2:25 ` [RFC PATCH v2 1/2] " Tsukasa OI 2023-08-10 2:25 ` [RFC PATCH v2 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 0/2] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only Tsukasa OI 4 siblings, 2 replies; 28+ messages in thread From: Tsukasa OI @ 2023-08-10 2:25 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, Jeff Law Cc: gcc-patches **WARNING** Following patch sets are exclusive: 1. [RFC PATCH v2] RISC-V: __builtin_riscv_pause for all environment (this) 2. [RFC PATCH] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only See <https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626748.html> for the background of this patch set. Changes: v1 -> v2 * Improve test case to test both RV32 and RV64 (+'Zihintpause'). Comparison: Patch sets [1] (this) and [2] * [1] completely preserves the compatibility with GCC 13 ([2] removes __builtin_riscv_pause if the 'Zihintpause' extension is absent, making a minor compatibility issue) * Because of the nature of the instrinsic, [2] is more natural ("pause" is defined in the 'Zihintpause' extension). Please consider those patch sets and decide which to apply. Sincerely, Tsukasa Tsukasa OI (2): RISC-V: __builtin_riscv_pause for all environment RISC-V: Fix documentation of __builtin_riscv_pause 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/doc/extend.texi | 6 +++--- gcc/testsuite/gcc.target/riscv/builtin_pause.c | 10 ---------- gcc/testsuite/gcc.target/riscv/zihintpause-1.c | 11 +++++++++++ gcc/testsuite/gcc.target/riscv/zihintpause-2.c | 11 +++++++++++ gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c | 12 ++++++++++++ 9 files changed, 51 insertions(+), 16 deletions(-) delete mode 100644 gcc/testsuite/gcc.target/riscv/builtin_pause.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-2.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c base-commit: 9b099a83b45b8fcdfc07d518e05d36ea741b2227 -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v2 1/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-10 2:25 ` [RFC PATCH v2 " Tsukasa OI @ 2023-08-10 2:25 ` Tsukasa OI 2023-08-16 1:26 ` Jeff Law 2023-08-10 2:25 ` [RFC PATCH v2 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI 1 sibling, 1 reply; 28+ messages in thread From: Tsukasa OI @ 2023-08-10 2:25 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, Jeff Law Cc: gcc-patches 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-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. --- 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-1.c | 11 +++++++++++ gcc/testsuite/gcc.target/riscv/zihintpause-2.c | 11 +++++++++++ gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c | 12 ++++++++++++ 8 files changed, 48 insertions(+), 13 deletions(-) delete mode 100644 gcc/testsuite/gcc.target/riscv/builtin_pause.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-2.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-noarch.c diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc index f25131eab28b..b77fdb909567 100644 --- a/gcc/common/config/riscv/riscv-common.cc +++ b/gcc/common/config/riscv/riscv-common.cc @@ -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}, @@ -1343,6 +1344,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 79681d759628..554fb7f69bb0 100644 --- a/gcc/config/riscv/riscv-builtins.cc +++ b/gcc/config/riscv/riscv-builtins.cc @@ -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 diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h index 28d9b81bd800..a6c3e0c9098f 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 688fd697255b..a6cdb32e9408 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -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\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 9250937cabb9..000000000000 --- 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 000000000000..fc86efe55902 --- /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 000000000000..4eaca95e9f02 --- /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 000000000000..7ce5cba90d51 --- /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 } } */ -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH v2 1/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-10 2:25 ` [RFC PATCH v2 1/2] " Tsukasa OI @ 2023-08-16 1:26 ` Jeff Law 2023-08-16 8:33 ` Philipp Tomsich 0 siblings, 1 reply; 28+ messages in thread From: Jeff Law @ 2023-08-16 1:26 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson Cc: gcc-patches On 8/9/23 20:25, Tsukasa OI 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-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. So the conclusion from today's meeting was to make this available irrespective of the extension set. So I've dropped the alternate patch from patchwork. > diff --git a/gcc/config/riscv/riscv-builtins.cc b/gcc/config/riscv/riscv-builtins.cc > index 79681d759628..554fb7f69bb0 100644 > --- a/gcc/config/riscv/riscv-builtins.cc > +++ b/gcc/config/riscv/riscv-builtins.cc > @@ -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 > diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h > index 28d9b81bd800..a6c3e0c9098f 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 688fd697255b..a6cdb32e9408 100644 > --- a/gcc/config/riscv/riscv.md > +++ b/gcc/config/riscv/riscv.md > @@ -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\t0x0100000f") > + So I was wondering if we'd be better off always emitting the .insn form with a comment on the line indicating it's a pause. ie something like .insn\t0x0100000f ;; pause That would allow the implementation to simplify down to a single unconditional pattern as well as simplifications elsewhere. Alternately we could do: TARGET_ZIHINTPAUSE ? pause : .insn\t0x0100000f in a single pattern that is always available if you feel strongly that we should emit different code based on TARGET_ZIHINTPAUSE. I think both simplify the riscv-builtins.cc change a bit. Thoughts? jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH v2 1/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-16 1:26 ` Jeff Law @ 2023-08-16 8:33 ` Philipp Tomsich 2023-08-21 14:02 ` Jeff Law 0 siblings, 1 reply; 28+ messages in thread From: Philipp Tomsich @ 2023-08-16 8:33 UTC (permalink / raw) To: Jeff Law Cc: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, gcc-patches On Wed, 16 Aug 2023 at 03:27, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > On 8/9/23 20:25, Tsukasa OI 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-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. > So the conclusion from today's meeting was to make this available > irrespective of the extension set. So I've dropped the alternate patch > from patchwork. > > > > diff --git a/gcc/config/riscv/riscv-builtins.cc b/gcc/config/riscv/riscv-builtins.cc > > index 79681d759628..554fb7f69bb0 100644 > > --- a/gcc/config/riscv/riscv-builtins.cc > > +++ b/gcc/config/riscv/riscv-builtins.cc > > @@ -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 > > diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h > > index 28d9b81bd800..a6c3e0c9098f 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 688fd697255b..a6cdb32e9408 100644 > > --- a/gcc/config/riscv/riscv.md > > +++ b/gcc/config/riscv/riscv.md > > @@ -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\t0x0100000f") > > + > So I was wondering if we'd be better off always emitting the .insn form > with a comment on the line indicating it's a pause. ie something like > > .insn\t0x0100000f ;; pause > > That would allow the implementation to simplify down to a single > unconditional pattern as well as simplifications elsewhere. > > Alternately we could do: > > TARGET_ZIHINTPAUSE ? pause : .insn\t0x0100000f Could we use the underlying 'fence' instruction (unless the assembler rejects the specific form that is needed) instead of the hex-insn? Should this also check HAVE_AS_MARCH_ZIHINTPAUSE (which must then also be added to configure.ac)? Philipp. > in a single pattern that is always available if you feel strongly that > we should emit different code based on TARGET_ZIHINTPAUSE. > > I think both simplify the riscv-builtins.cc change a bit. > > Thoughts? > > jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC PATCH v2 1/2] RISC-V: __builtin_riscv_pause for all environment 2023-08-16 8:33 ` Philipp Tomsich @ 2023-08-21 14:02 ` Jeff Law 0 siblings, 0 replies; 28+ messages in thread From: Jeff Law @ 2023-08-21 14:02 UTC (permalink / raw) To: Philipp Tomsich Cc: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, gcc-patches On 8/16/23 02:33, Philipp Tomsich wrote: > > Could we use the underlying 'fence' instruction (unless the assembler > rejects the specific form that is needed) instead of the hex-insn? > Should this also check HAVE_AS_MARCH_ZIHINTPAUSE (which must then also > be added to configure.ac)? It seems reasonable from an encoding standpoint. But I haven't been able to convince the assembler to encode a 0 into the pred/succ fields. Jeff ^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH v2 2/2] RISC-V: Fix documentation of __builtin_riscv_pause 2023-08-10 2:25 ` [RFC PATCH v2 " Tsukasa OI 2023-08-10 2:25 ` [RFC PATCH v2 1/2] " Tsukasa OI @ 2023-08-10 2:25 ` Tsukasa OI 1 sibling, 0 replies; 28+ messages in thread From: Tsukasa OI @ 2023-08-10 2:25 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, Jeff Law Cc: gcc-patches From: Tsukasa OI <research_trasio@irq.a4lg.com> This built-in does not imply the 'Xgnuzihintpausestate' extension. It does not change architectural state (because all HINTs are prohibited from doing that). gcc/ChangeLog: * doc/extend.texi: Fix the description of __builtin_riscv_pause. --- gcc/doc/extend.texi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 89c5b4ea2b20..7ebbe70c78d6 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -21570,9 +21570,9 @@ Returns the value that is currently set in the @samp{tp} register. @enddefbuiltin @defbuiltin{void __builtin_riscv_pause (void)} -Generates the @code{pause} (hint) machine instruction. This implies the -Xgnuzihintpausestate extension, which redefines the @code{pause} instruction to -change architectural state. +Generates the @code{pause} (hint) machine instruction. If the target implements +the Zihintpause extension, it indicates that the current hart should be +temporarily paused or slowed down. @enddefbuiltin @node RISC-V Vector Intrinsics -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH 0/2] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only 2023-08-09 6:11 [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Tsukasa OI ` (3 preceding siblings ...) 2023-08-10 2:25 ` [RFC PATCH v2 " Tsukasa OI @ 2023-08-10 2:26 ` Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 1/2] " Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI 4 siblings, 2 replies; 28+ messages in thread From: Tsukasa OI @ 2023-08-10 2:26 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, Jeff Law Cc: gcc-patches **WARNING** Following patch sets are exclusive: 1. [RFC PATCH v2] RISC-V: __builtin_riscv_pause for all environment 2. [RFC PATCH] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only (this) See <https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626748.html> for the background of this patch set. Changes: (all environment) v1 -> ('Zihintpause'-only) v1 * Based on the feedback from Jeff Law, I made an alternative patch set which makes "__builtin_riscv_pause" 'Zihintpause'-only. Comparison: Patch sets [1] and [2] (this) * [1] completely preserves the compatibility with GCC 13 ([2] removes __builtin_riscv_pause if the 'Zihintpause' extension is absent, making a minor compatibility issue) * Because of the nature of the instrinsic, [2] is more natural ("pause" is defined in the 'Zihintpause' extension). Please consider those patch sets and decide which to apply. Sincerely, Tsukasa Tsukasa OI (2): RISC-V: Make __builtin_riscv_pause 'Zihintpause' only RISC-V: Fix documentation of __builtin_riscv_pause gcc/common/config/riscv/riscv-common.cc | 2 ++ gcc/config/riscv/riscv-builtins.cc | 4 ++-- gcc/config/riscv/riscv-opts.h | 2 ++ gcc/config/riscv/riscv.md | 2 +- gcc/doc/extend.texi | 6 +++--- gcc/testsuite/gcc.target/riscv/builtin_pause.c | 10 ---------- gcc/testsuite/gcc.target/riscv/zihintpause-1.c | 11 +++++++++++ gcc/testsuite/gcc.target/riscv/zihintpause-2.c | 11 +++++++++++ 8 files changed, 32 insertions(+), 16 deletions(-) delete mode 100644 gcc/testsuite/gcc.target/riscv/builtin_pause.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-2.c base-commit: 9b099a83b45b8fcdfc07d518e05d36ea741b2227 -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH 1/2] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only 2023-08-10 2:26 ` [RFC PATCH 0/2] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only Tsukasa OI @ 2023-08-10 2:26 ` Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI 1 sibling, 0 replies; 28+ messages in thread From: Tsukasa OI @ 2023-08-10 2:26 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, Jeff Law Cc: gcc-patches 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. Despite that the "pause" instruction code (0x0100000f) is a HINT and emitting its instruction code is safe in any environment, enabling the built-in only for certain environment is not a good idea. This commit implements handling for the 'Zihintpause' extension and makes __built_riscv_pause built-in 'Zihintpause'-only. 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", corresponding the existence of the 'Zihintpause' extension. (riscv_builtins) Change che "riscv_pause" requirements. * 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. 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/common/config/riscv/riscv-common.cc | 2 ++ gcc/config/riscv/riscv-builtins.cc | 4 ++-- gcc/config/riscv/riscv-opts.h | 2 ++ gcc/config/riscv/riscv.md | 2 +- gcc/testsuite/gcc.target/riscv/builtin_pause.c | 10 ---------- gcc/testsuite/gcc.target/riscv/zihintpause-1.c | 11 +++++++++++ gcc/testsuite/gcc.target/riscv/zihintpause-2.c | 11 +++++++++++ 7 files changed, 29 insertions(+), 13 deletions(-) delete mode 100644 gcc/testsuite/gcc.target/riscv/builtin_pause.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-1.c create mode 100644 gcc/testsuite/gcc.target/riscv/zihintpause-2.c diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc index f25131eab28b..b77fdb909567 100644 --- a/gcc/common/config/riscv/riscv-common.cc +++ b/gcc/common/config/riscv/riscv-common.cc @@ -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}, @@ -1343,6 +1344,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 79681d759628..73e1512a5992 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, TARGET_ZIHINTPAUSE) /* 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), + DIRECT_NO_TARGET_BUILTIN (pause, 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 28d9b81bd800..a6c3e0c9098f 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 688fd697255b..44ea1c54b96b 100644 --- a/gcc/config/riscv/riscv.md +++ b/gcc/config/riscv/riscv.md @@ -2192,7 +2192,7 @@ (define_insn "riscv_pause" [(unspec_volatile [(const_int 0)] UNSPECV_PAUSE)] - "" + "TARGET_ZIHINTPAUSE" "pause") ;; 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 9250937cabb9..000000000000 --- 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 000000000000..fc86efe55902 --- /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 000000000000..4eaca95e9f02 --- /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 } } */ -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause 2023-08-10 2:26 ` [RFC PATCH 0/2] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 1/2] " Tsukasa OI @ 2023-08-10 2:26 ` Tsukasa OI 1 sibling, 0 replies; 28+ messages in thread From: Tsukasa OI @ 2023-08-10 2:26 UTC (permalink / raw) To: Tsukasa OI, Kito Cheng, Palmer Dabbelt, Andrew Waterman, Jim Wilson, Jeff Law Cc: gcc-patches From: Tsukasa OI <research_trasio@irq.a4lg.com> This built-in does not imply the 'Xgnuzihintpausestate' extension. It does not change architectural state (because all HINTs are prohibited from doing that). gcc/ChangeLog: * doc/extend.texi: Fix the description of __builtin_riscv_pause. --- gcc/doc/extend.texi | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 89c5b4ea2b20..7be27430666a 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -21570,9 +21570,9 @@ Returns the value that is currently set in the @samp{tp} register. @enddefbuiltin @defbuiltin{void __builtin_riscv_pause (void)} -Generates the @code{pause} (hint) machine instruction. This implies the -Xgnuzihintpausestate extension, which redefines the @code{pause} instruction to -change architectural state. +Generates the @code{pause} (hint) machine instruction, indicating that the +current hart should be temporarily paused or slowed down. +Available if the Zihintpause extension is enabled. @enddefbuiltin @node RISC-V Vector Intrinsics -- 2.41.0 ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2023-08-29 9:26 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-09 6:11 [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Tsukasa OI 2023-08-09 6:11 ` [RFC PATCH 1/2] " Tsukasa OI 2023-08-28 21:12 ` Jeff Law 2023-08-29 2:02 ` Tsukasa OI 2023-08-09 6:11 ` [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI 2023-08-28 21:14 ` Jeff Law 2023-08-28 23:09 ` Hans-Peter Nilsson 2023-08-28 23:15 ` Jeff Law 2023-08-29 2:15 ` Tsukasa OI 2023-08-29 9:26 ` Hans-Peter Nilsson 2023-08-09 20:05 ` [RFC PATCH 0/2] RISC-V: __builtin_riscv_pause for all environment Jeff Law 2023-08-09 22:39 ` Tsukasa OI 2023-08-11 23:30 ` Jeff Law 2023-08-11 23:34 ` Palmer Dabbelt 2023-08-12 0:20 ` Tsukasa OI 2023-08-15 13:49 ` Jeff Law 2023-08-13 19:52 ` Philipp Tomsich 2023-08-13 20:17 ` Andrew Waterman 2023-08-15 13:52 ` Jeff Law 2023-08-10 2:25 ` [RFC PATCH v2 " Tsukasa OI 2023-08-10 2:25 ` [RFC PATCH v2 1/2] " Tsukasa OI 2023-08-16 1:26 ` Jeff Law 2023-08-16 8:33 ` Philipp Tomsich 2023-08-21 14:02 ` Jeff Law 2023-08-10 2:25 ` [RFC PATCH v2 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 0/2] RISC-V: Make __builtin_riscv_pause 'Zihintpause' only Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 1/2] " Tsukasa OI 2023-08-10 2:26 ` [RFC PATCH 2/2] RISC-V: Fix documentation of __builtin_riscv_pause Tsukasa OI
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).