RFR: 8362193: Re-work MacOS/AArch64 SpinPause to handle SB [v3]

Evgeny Astigeevich eastigeevich at openjdk.org
Mon Jul 28 16:33:07 UTC 2025


On Mon, 21 Jul 2025 20:31:27 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> ### Background
>> 
>> With JDK-8359435 "AArch64: add support for SB instruction to MacroAssembler::spin_wait" we have an option to use the speculation barrier (SB) instruction for `j.l.Thread::onSpinWait` and `SpinPause`. On Linux AArch64 `SpinPause` uses a stub which is generated  with `MacroAssembler::spin_wait`. `j.l.Thread::onSpinWait` uses it as well. As a result tests for `j.l.Thread::onSpinWait`, e.g.`compiler/onSpinWait/TestOnSpinWaitAArch64.java`, cover both `j.l.Thread::onSpinWait` and `SpinPause`. Also we don't need to update `SpinPause` when we add support for a new instruction to `MacroAssembler::spin_wait`.
>> 
>> On Mac AArch64 `SpinPause` does not use the generated stub to  avoid a costly call to `os::current_thread_enable_wx()`. It uses inline assembly instead. As a result we have `SpinWait` implementation details leaking into `SpinPause`. Besides asserts there are no tests covering Mac implementation of `SpinPause`. Testing on Apple M3 Pro showed that `compiler/onSpinWait/TestOnSpinWaitAArch64.java` could not reliably trigger those asserts. `ArchiveWorkers::run_task_multi` did not invoke `spin.wait()`. The inline assembly in Mac AArch64 `SpinPause` has another issue. It uses a jump table. An offset in the table is calculated based on the value of `enum SpinWait::Inst`. When a new value `SB` was added the offset became out of bounds. The jump goes out of the assembly code.
>> 
>> ### Summary of changes
>> 
>> * Rewrote Mac AArch64 `SpinPause` not to use a jump table.
>> * Added `SpinWait::supports()` and `SpinWait::from_name()` methods to validate and convert instruction names into corresponding enum values.
>> * Updated `SpinWait::Inst` enum to use bit flags for simplified assembly testing and added a constructor to initialize `SpinWait` using instruction names.
>> * `src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp`: Added an assertion to ensure the `sb` instruction is supported by the CPU before generation.
>> * Added `OnSpinWaitInstNameConstraintFunc` for the `OnSpinWaitInst` option to check option's value at parsing.
>> * Added a gtest to verify the functionality of `SpinPause`.
>> * Added a jtreg test to run the gtest for`SpinPause` with various instructions, including the `sb` instruction if supported by the CPU.
>> 
>> ### Testing results: fastdebug, release
>> 
>> - Linux, Graviton 2
>>    - tier1: Passed
>>    - `test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java`: Passed
>>    - `test/hotspot/jtreg/compiler/onSpinWait`: Passed
>> - MacOS, M3 Pro
> ...
>
> Evgeny Astigeevich has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Put cases in enum order

At least we can test armv8.7 and armv9.2 features on M4.  The next Neoverse supporting armv9.2 will be N3/V3. Taking into account Neoverse N2/V2 hardware is quite fresh, we might not seen N3/V3 hardware soon.

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

PR Comment: https://git.openjdk.org/jdk/pull/26387#issuecomment-3128052931


More information about the hotspot-dev mailing list