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 >