RFR: 8285301: C2: assert(!requires_atomic_access) failed: can't ensure atomicity [v2]

[Tobias Hartmann]

On Thu, 28 Apr 2022 05:30:30 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> We hit asserts in `BarrierSetC2::load_at_resolved` and `BarrierSetC2::store_at_resolved` when running with `-XX:+AlwaysAtomicAccesses` because the corresponding code paths have not been implemented yet. Although the assert triggers on all platforms, it only affects long and double accesses on 32-bit systems (everything else is atomic anyway).
>> 
>> I moved the `requires_atomic_access` logic into `LoadNode::make` and `StoreNode::make` and refactored related code.
>> 
>> I noticed that `LoadNode::convert_to_reinterpret_load` and `StoreNode::convert_to_reinterpret_store` did not properly check for `requires_atomic_access` and fixed that as well.
>> 
>> I'm currently running all tests with `-XX:+AlwaysAtomicAccesses`.
>> 
>> Thanks,
>> Tobias
>
> Tobias Hartmann has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Removed default arg
>  - Invoking Opcode only once. Added comment to test

Thanks for the reviews, Vladimir and Dean.

> The new TestAlwaysAtomicAccesses test seems suspiciously simple. I guess the idea is to catch issues during startup with -Xcomp. A comment saying that could prevent someone from thinking the test does nothing.

Right, we have quite a few such tests for flags that are rarely used. A simple run with `-Xcomp` is enough to trigger the originally report issues. More issues are then triggered by the other tests that I modified. I added a comment explaining the purpose.

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

PR: https://git.openjdk.java.net/jdk/pull/8422