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