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
>
