RFR: 8315841: RISC-V: Check for hardware TSO support [v4]
Robbin Ehn
rehn at openjdk.org
Thu Sep 7 13:14:32 UTC 2023
On Thu, 7 Sep 2023 12:58:12 GMT, Ludovic Henry <luhenry at openjdk.org> wrote:
>> src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 381:
>>
>>> 379: void fence(uint32_t predecessor, uint32_t successor) {
>>> 380: if (UseZtso) {
>>> 381: if (pred_succ_to_membar_mask(predecessor, successor) & StoreLoad) {
>>
>> This should be "(pred_succ_to_membar_mask(predecessor, successor) & StoreLoad) == 0".
>
> Not AFAIU, as we would then _not_ generate `StoreLoad` barriers which are the only one TSO doesn't guarantee.
No, sorry I mean != 0. We don't if on integer values.
So that if should be a boolean expression.
>> src/hotspot/cpu/riscv/vm_version_riscv.cpp line 213:
>>
>>> 211: }
>>> 212:
>>> 213: #if defined(TARGET_ZTSO) && TARGET_ZTSO
>>
>> If someone compiles with "CXXFLAGS=-marchrv64....ztso..", we need to try to parse the supplied flags, that doesn't seem like fun.
>> Instead I suggest we add code to read-out the elf flags, i.e:
>> "Flags: 0x15, RVC, double-float ABI, TSO"
>>
>> And set UseZtso:
>> A: If this is a TSO elf.
>> B: If hwprobe says this TSO hardware (either directly or via vendor).
>> C: If someone set flag,
>>
>> I guess your idea was to have a flag like --enable-tso which sets TARGET_TSO ?
>> If we have that or not I still like above to happen.
>>
>> (I'm not saying you should do any of this in this PR, I can file new ones)
>
> `TARGET_TSO` is set by gcc directly. See https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281514.html
Ah, it is not set by LLVM what I can see at least (running a couple of weeks old tip).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15613#discussion_r1318570723
PR Review Comment: https://git.openjdk.org/jdk/pull/15613#discussion_r1318578037
More information about the hotspot-dev
mailing list