RFR: 8316566: RISC-V: Zero extended narrow oop passed to Atomic::cmpxchg

Robbin Ehn rehn at openjdk.org
Wed Sep 27 07:54:11 UTC 2023


On Tue, 26 Sep 2023 14:08:54 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hi, please consider!
>> 
>> There is bug in gcc < 12 where __synch_synchronize() in some corner-cases don't enforce the compiler barrier.
>> This causes some code to be placed after the __synch_synchronize(), and in this case causing a word to to be not be sign extended as a collateral issue of the bug.
>> You can see the 'bad' assembly in JBS, where a branch is moved over the compiler barrier.
>> 
>> Trying to get information from gcc folks.
>> 
>> It seems like either adding a extra compiler barrier, or use  __atomic_thread_fence(__ATOMIC_SEQ_CST) fixes it.
>> 
>> Tested https://bugs.openjdk.org/browse/JDK-8316186 with this fix.
>> Manually verified assembly, with this fix we generate the same as gcc 12.
>
> Good catch! But seems to me that it is not necessarily be a missing compiler barrier issue here. 
> The 'bne' branch instruction is supposed to be there in the critical section which should be similar as our inline asm [1]. 
> I guess there might be some other gcc optimization bug which erroneously deleted the necessary 32-bit sign-extension instruction. 
> So better to hear what the gcc folks say I think. 
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/os_cpu/linux_riscv/atomic_linux_riscv.hpp#L116

Thanks @RealFYang !

I'll stale a day or two and see if compiler can give some more hints before push.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15917#issuecomment-1736885501


More information about the hotspot-runtime-dev mailing list