RFR: 8351997: AArch64: Interpreter volatile reference stores with G1 are not sequentially consistent [v2]

[Erik Österlund]

On Mon, 2 Jun 2025 07:48:39 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> The optimized fast_aputfield bytecode on AArch64 stores the field flags in r3, and performs the leading and trailing fencing depending on its volatile bit being set or not. However, r3 is also the last temp register passed in to the barrier set for reference stores, and G1 clobbers it in a way that may clear the volatile bit. Then the trailing fence won't get executed, and sequential consistency is broken.
>> 
>> My fix puts the flags in r5 instead, which is the register that was used by normal aputfield bytecodes. This way, barriers don't clobber the volatile bits.
>> 
>> This bug has been observed to mess up a classic Dekker duality in the java.util.concurrent.Exchanger class, leading to a hang in the test/jdk/java/util/concurrent/Exchanger/ExchangeLoops.java test that exercises it. Using G1 and -Xint a reproducer hangs 30/100 times in mach5. With the fix, the same reproducer hangs 0/100 times.
>
> Erik Österlund has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add comment about clobbered registers

> Well, since we are introducing the hunks near `do_oop_store`-s, and thus extending the scope of the patch. At this point, we can just inline `do_oop_store` (and maybe `do_oop_load`?), like Andrew initially suggested. This will also match what RISC-V already did: [c5a1543](https://github.com/openjdk/jdk/commit/c5a1543ee3e68775f09ca29fb07efd9aebfdb33e)

RISC-V doesn't really have the backporting until JDK 8 problem. I'd really like to make that cosmetic change in the next follow-up PR instead, as previously discussed. The comments hold true all the way back to JDK 8 and don't change the logic, so I can go along with that. And I'd rather take the risk of getting some comment wrong on the way back to JDK 8, than fiddling with the guts of all this unrelated code, that has changed substantially since back then. Does that sound okay?

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

PR Comment: https://git.openjdk.org/jdk/pull/25483#issuecomment-2929515436