From ae46cf83d13b7738811350d017f6977bff379182 Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 11:34:47 +0000 Subject: [PATCH 01/13] 8362193: compiler/onSpinWait/TestOnSpinWaitAArch64.java asserts on spin-wait insns range checks --- src/hotspot/cpu/aarch64/globals_aarch64.hpp | 6 +- .../cpu/aarch64/macroAssembler_aarch64.cpp | 20 +--- src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp | 51 ++++++++++ src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp | 25 +++-- .../cpu/aarch64/spin_wait_aarch64.inline.hpp | 96 +++++++++++++++++++ .../cpu/aarch64/vm_version_aarch64.cpp | 26 ++--- .../os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 33 +------ .../hotspot/gtest/aarch64/test_spin_pause.cpp | 33 +++++++ .../jtreg/gtest/TestSpinPauseAArch64.java | 37 +++++++ .../jtreg/gtest/TestSpinPauseSBAArch64.java | 34 +++++++ 10 files changed, 288 insertions(+), 73 deletions(-) create mode 100644 src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp create mode 100644 src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp create mode 100644 test/hotspot/gtest/aarch64/test_spin_pause.cpp create mode 100644 test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java create mode 100644 test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java diff --git a/src/hotspot/cpu/aarch64/globals_aarch64.hpp b/src/hotspot/cpu/aarch64/globals_aarch64.hpp index ef741c2007adf..0ddccf1666489 100644 --- a/src/hotspot/cpu/aarch64/globals_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/globals_aarch64.hpp @@ -26,6 +26,7 @@ #ifndef CPU_AARCH64_GLOBALS_AARCH64_HPP #define CPU_AARCH64_GLOBALS_AARCH64_HPP +#include "spin_wait_aarch64.hpp" #include "utilities/globalDefinitions.hpp" #include "utilities/macros.hpp" @@ -114,10 +115,11 @@ define_pd_global(intx, InlineSmallCode, 1000); "Use prfm hint with specified distance in compiled code." \ "Value -1 means off.") \ range(-1, 4096) \ - product(ccstr, OnSpinWaitInst, "yield", DIAGNOSTIC, \ + product(ccstr, OnSpinWaitInst, DEFAULT_SPIN_WAIT_INST, DIAGNOSTIC, \ "The instruction to use to implement " \ "java.lang.Thread.onSpinWait()." \ - "Options: none, nop, isb, yield, sb.") \ + "Options: " \ + SPIN_WAIT_INST_OPTIONS) \ product(uint, OnSpinWaitInstCount, 1, DIAGNOSTIC, \ "The number of OnSpinWaitInst instructions to generate." \ "It cannot be used with OnSpinWaitInst=none.") \ diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index e8290ae10caa7..c50b9de4a161e 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -54,6 +54,7 @@ #include "runtime/jniHandles.inline.hpp" #include "runtime/sharedRuntime.hpp" #include "runtime/stubRoutines.hpp" +#include "spin_wait_aarch64.inline.hpp" #include "utilities/globalDefinitions.hpp" #include "utilities/powerOfTwo.hpp" #ifdef COMPILER1 @@ -6804,24 +6805,7 @@ void MacroAssembler::verify_cross_modify_fence_not_required() { void MacroAssembler::spin_wait() { block_comment("spin_wait {"); - for (int i = 0; i < VM_Version::spin_wait_desc().inst_count(); ++i) { - switch (VM_Version::spin_wait_desc().inst()) { - case SpinWait::NOP: - nop(); - break; - case SpinWait::ISB: - isb(); - break; - case SpinWait::YIELD: - yield(); - break; - case SpinWait::SB: - sb(); - break; - default: - ShouldNotReachHere(); - } - } + generate_spin_wait(this, VM_Version::spin_wait_desc()); block_comment("}"); } diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp new file mode 100644 index 0000000000000..ac926914ffd61 --- /dev/null +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp @@ -0,0 +1,51 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +#include "spin_wait_aarch64.hpp" + +#include + +bool SpinWait::supports(const char *name) { + return name != nullptr && + (strcmp(name, "nop") == 0 || + strcmp(name, "isb") == 0 || + strcmp(name, "yield") == 0 || + strcmp(name, "sb") == 0 || + strcmp(name, "none") == 0); +} + +SpinWait::Inst SpinWait::from_name(const char* name) { + assert(supports(name), "spin wait instruction name must be one of: " SPIN_WAIT_INST_OPTIONS); + + if (strcmp(name, "nop") == 0) { + return SpinWait::NOP; + } else if (strcmp(name, "isb") == 0) { + return SpinWait::ISB; + } else if (strcmp(name, "yield") == 0) { + return SpinWait::YIELD; + } else if (strcmp(name, "sb") == 0) { + return SpinWait::SB; + } + + return SpinWait::NONE; +} diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp index 08850f05f530c..71ce4e0689022 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp @@ -19,31 +19,42 @@ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA * or visit www.oracle.com if you need additional information or have any * questions. - * */ #ifndef CPU_AARCH64_SPIN_WAIT_AARCH64_HPP #define CPU_AARCH64_SPIN_WAIT_AARCH64_HPP +#define DEFAULT_SPIN_WAIT_INST "yield" +#define SPIN_WAIT_INST_OPTIONS "nop, isb, yield, sb, none" + class SpinWait { public: + // Non-zero values are chosen to have only one bit set. + // This simplifies testing values in assembly code. + // This limits us to 64 possible implementation. + // Value 1 is used for the default implementation. enum Inst { - NONE = -1, - NOP, - ISB, - YIELD, - SB + NONE = 0, + YIELD = (1 << 0), + ISB = (1 << 1), + SB = (1 << 2), + NOP = (1 << 3) }; private: Inst _inst; int _count; + Inst from_name(const char *name); + public: - SpinWait(Inst inst = NONE, int count = 0) : _inst(inst), _count(count) {} + SpinWait(Inst inst = NONE, int count = 0) : _inst(inst), _count(inst == NONE ? 0 : count) {} + SpinWait(const char *name, int count) : SpinWait(from_name(name), count) {} Inst inst() const { return _inst; } int inst_count() const { return _count; } + + static bool supports(const char *name); }; #endif // CPU_AARCH64_SPIN_WAIT_AARCH64_HPP diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp new file mode 100644 index 0000000000000..9cf8f90c1a127 --- /dev/null +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp @@ -0,0 +1,96 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +#ifndef CPU_AARCH64_SPIN_WAIT_AARCH64_INLINE_HPP +#define CPU_AARCH64_SPIN_WAIT_AARCH64_INLINE_HPP + +#include "asm/macroAssembler.hpp" +#include "spin_wait_aarch64.hpp" +#include "utilities/powerOfTwo.hpp" + +inline void exec_spin_wait_inst(SpinWait::Inst inst_id) { + assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or use only one bit"); + assert(SpinWait::YIELD == 1, "SpinWait::Inst value 1 reserved for 'yield' instruction"); + assert(SpinWait::ISB == 2, "SpinWait::Inst value 2 reserved for 'isb' instruction"); + assert(SpinWait::SB == 4, "SpinWait::Inst value 4 reserved for 'sb' instruction"); + assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); + + // The assembly code below is equivalent to the following: + // + // if (inst_id == 1) { + // exec_yield_inst(); + // } else if (inst_id == 2) { + // exec_isb_inst(); + // } else if (inst_id == 4) { + // exec_sb_inst(); + // } else if (inst_id == 8) { + // exec_nop_inst(); + // } + // + // New values don't break the code. They will have an effect of NONE. + asm volatile( + " tbz %[id], 0, 0f \n" // The default instruction for SpinWait is YIELD. + // We check it first before going to switch. + " yield \n" + " b 4f \n" + "0: \n" + " tbnz %[id], 1, 1f \n" + " tbnz %[id], 2, 2f \n" + " tbnz %[id], 3, 3f \n" + " b 4f \n" + "1: \n" + " isb \n" + " b 4f \n" + "2: \n" + " .inst 0xd50330ff \n" // SB instruction, explicitly encoded not to rely on a compiler + " b 4f \n" + "3: \n" + " nop \n" + "4: \n" + : + : [id]"r"((uint64_t)inst_id) + : "memory"); +} + +inline void generate_spin_wait(MacroAssembler *masm, const SpinWait &spin_wait_desc) { + for (int i = 0; i < spin_wait_desc.inst_count(); ++i) { + switch (spin_wait_desc.inst()) { + case SpinWait::NOP: + masm->nop(); + break; + case SpinWait::ISB: + masm->isb(); + break; + case SpinWait::YIELD: + masm->yield(); + break; + case SpinWait::SB: + masm->sb(); + break; + default: + ShouldNotReachHere(); + } + } +} + +#endif // CPU_AARCH64_SPIN_WAIT_AARCH64_INLINE_HPP diff --git a/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp b/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp index 6ee4a0023c699..f70e66ba2f7ac 100644 --- a/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp @@ -51,26 +51,20 @@ uintptr_t VM_Version::_pac_mask; SpinWait VM_Version::_spin_wait; static SpinWait get_spin_wait_desc() { - if (strcmp(OnSpinWaitInst, "nop") == 0) { - return SpinWait(SpinWait::NOP, OnSpinWaitInstCount); - } else if (strcmp(OnSpinWaitInst, "isb") == 0) { - return SpinWait(SpinWait::ISB, OnSpinWaitInstCount); - } else if (strcmp(OnSpinWaitInst, "yield") == 0) { - return SpinWait(SpinWait::YIELD, OnSpinWaitInstCount); - } else if (strcmp(OnSpinWaitInst, "sb") == 0) { - if (!VM_Version::supports_sb()) { - vm_exit_during_initialization("OnSpinWaitInst is SB but current CPU does not support SB instruction"); - } - return SpinWait(SpinWait::SB, OnSpinWaitInstCount); - } else if (strcmp(OnSpinWaitInst, "none") != 0) { - vm_exit_during_initialization("The options for OnSpinWaitInst are nop, isb, yield, sb, and none", OnSpinWaitInst); + if (!SpinWait::supports(OnSpinWaitInst)) { + vm_exit_during_initialization("OnSpinWaitInst is not one of " + SPIN_WAIT_INST_OPTIONS, + OnSpinWaitInst); } - if (!FLAG_IS_DEFAULT(OnSpinWaitInstCount) && OnSpinWaitInstCount > 0) { - vm_exit_during_initialization("OnSpinWaitInstCount cannot be used for OnSpinWaitInst 'none'"); + assert(OnSpinWaitInstCount != 0, "allowed range for OnSpinWaitInstCount must not include 0"); + + SpinWait spin_wait(OnSpinWaitInst, OnSpinWaitInstCount); + if (spin_wait.inst() == SpinWait::SB && !VM_Version::supports_sb()) { + vm_exit_during_initialization("OnSpinWaitInst is SB but current CPU does not support SB instruction"); } - return SpinWait{}; + return spin_wait; } void VM_Version::initialize() { diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index b7556ca69da79..fe993bbbd0871 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -50,6 +50,7 @@ #include "runtime/stubRoutines.hpp" #include "runtime/timer.hpp" #include "signals_posix.hpp" +#include "spin_wait_aarch64.inline.hpp" #include "utilities/align.hpp" #include "utilities/events.hpp" #include "utilities/vmError.hpp" @@ -524,40 +525,12 @@ static inline void atomic_copy64(const volatile void *src, volatile void *dst) { } extern "C" { - // needs local assembler label '1:' to avoid trouble when using linktime optimization int SpinPause() { // We don't use StubRoutines::aarch64::spin_wait stub in order to // avoid a costly call to os::current_thread_enable_wx() on MacOS. // We should return 1 if SpinPause is implemented, and since there - // will be a sequence of 11 instructions for NONE and YIELD and 12 - // instructions for NOP and ISB, SpinPause will always return 1. - uint64_t br_dst; - const int instructions_per_case = 2; - int64_t off = VM_Version::spin_wait_desc().inst() * instructions_per_case * Assembler::instruction_size; - - assert(VM_Version::spin_wait_desc().inst() >= SpinWait::NONE && - VM_Version::spin_wait_desc().inst() <= SpinWait::YIELD, "must be"); - assert(-1 == SpinWait::NONE, "must be"); - assert( 0 == SpinWait::NOP, "must be"); - assert( 1 == SpinWait::ISB, "must be"); - assert( 2 == SpinWait::YIELD, "must be"); - - asm volatile( - " adr %[d], 20 \n" // 20 == PC here + 5 instructions => address - // to entry for case SpinWait::NOP - " add %[d], %[d], %[o] \n" - " br %[d] \n" - " b 1f \n" // case SpinWait::NONE (-1) - " nop \n" // padding - " nop \n" // case SpinWait::NOP ( 0) - " b 1f \n" - " isb \n" // case SpinWait::ISB ( 1) - " b 1f \n" - " yield \n" // case SpinWait::YIELD ( 2) - "1: \n" - : [d]"=&r"(br_dst) - : [o]"r"(off) - : "memory"); + // will be always a sequence of instructions, SpinPause will always return 1. + exec_spin_wait(VM_Version::spin_wait_desc().inst()); return 1; } diff --git a/test/hotspot/gtest/aarch64/test_spin_pause.cpp b/test/hotspot/gtest/aarch64/test_spin_pause.cpp new file mode 100644 index 0000000000000..e220362eae95e --- /dev/null +++ b/test/hotspot/gtest/aarch64/test_spin_pause.cpp @@ -0,0 +1,33 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +#if defined(AARCH64) && !defined(ZERO) + +#include "utilities/spinYield.hpp" +#include "unittest.hpp" + +TEST_VM(SpinPause, sanity) { + ASSERT_EQ(SpinPause(), 1); +} + +#endif // AARCH64 diff --git a/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java b/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java new file mode 100644 index 0000000000000..15d796bc21c8d --- /dev/null +++ b/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java @@ -0,0 +1,37 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +/** + * @test TestSpinPauseAArch64 + * @bug 8362193 + * @summary Run SpinPause gtest using different instructions for SpinPause + * @library /test/lib + * @requires vm.flagless + * @requires os.arch=="aarch64" + * @run main/native GTestWrapper --gtest_filter=SpinPause* + * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=none + * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=nop + * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=isb + * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=yield + */ diff --git a/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java b/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java new file mode 100644 index 0000000000000..b05b10e9c1973 --- /dev/null +++ b/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java @@ -0,0 +1,34 @@ +/* + * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +/** + * @test TestSpinPauseAArch64 + * @bug 8362193 + * @summary Run SpinPause gtest using different instructions for SpinPause + * @library /test/lib + * @requires vm.flagless + * @requires os.arch=="aarch64" + * @requires vm.cpu.features ~= ".*sb.*" + * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=sb + */ From 3b3c969691c39f315db79b53f55ab028bead1812 Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 13:35:41 +0000 Subject: [PATCH 02/13] Correct call of exec_spin_wait_inst --- src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp | 11 ++++++++--- src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp index 9cf8f90c1a127..d3ad4c0232075 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp @@ -25,15 +25,22 @@ #define CPU_AARCH64_SPIN_WAIT_AARCH64_INLINE_HPP #include "asm/macroAssembler.hpp" +#include "runtime/vm_version.hpp" #include "spin_wait_aarch64.hpp" #include "utilities/powerOfTwo.hpp" inline void exec_spin_wait_inst(SpinWait::Inst inst_id) { - assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or use only one bit"); + assert(SpinWait::NONE == 0, "SpinWait::Inst value 0 reserved to indicate no implementation"); assert(SpinWait::YIELD == 1, "SpinWait::Inst value 1 reserved for 'yield' instruction"); assert(SpinWait::ISB == 2, "SpinWait::Inst value 2 reserved for 'isb' instruction"); assert(SpinWait::SB == 4, "SpinWait::Inst value 4 reserved for 'sb' instruction"); assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); + assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or use only one bit"); + assert(inst_id <= SpinWait::NOP, "Unsupported type of SpinWait::Inst: %d", inst_id); + + if (inst_id < SpinWait::NONE || inst_id > SpinWait::NOP) { + ShouldNotReachHere(); + } // The assembly code below is equivalent to the following: // @@ -46,8 +53,6 @@ inline void exec_spin_wait_inst(SpinWait::Inst inst_id) { // } else if (inst_id == 8) { // exec_nop_inst(); // } - // - // New values don't break the code. They will have an effect of NONE. asm volatile( " tbz %[id], 0, 0f \n" // The default instruction for SpinWait is YIELD. // We check it first before going to switch. diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index fe993bbbd0871..1e8d6d2dd6029 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -530,7 +530,7 @@ extern "C" { // avoid a costly call to os::current_thread_enable_wx() on MacOS. // We should return 1 if SpinPause is implemented, and since there // will be always a sequence of instructions, SpinPause will always return 1. - exec_spin_wait(VM_Version::spin_wait_desc().inst()); + exec_spin_wait_inst(VM_Version::spin_wait_desc().inst()); return 1; } From 60e08a3ba7fd512d45c4c1650b3d709fe3d9c36a Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 13:42:29 +0000 Subject: [PATCH 03/13] Assert SB support --- src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp | 2 ++ test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java | 1 - test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp index d3ad4c0232075..e896639730e25 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp @@ -37,6 +37,7 @@ inline void exec_spin_wait_inst(SpinWait::Inst inst_id) { assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or use only one bit"); assert(inst_id <= SpinWait::NOP, "Unsupported type of SpinWait::Inst: %d", inst_id); + assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); if (inst_id < SpinWait::NONE || inst_id > SpinWait::NOP) { ShouldNotReachHere(); @@ -90,6 +91,7 @@ inline void generate_spin_wait(MacroAssembler *masm, const SpinWait &spin_wait_d masm->yield(); break; case SpinWait::SB: + assert(VM_Version::supports_sb(), "current CPU does not support SB instruction"); masm->sb(); break; default: diff --git a/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java b/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java index 15d796bc21c8d..a998f6fec110b 100644 --- a/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java +++ b/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java @@ -19,7 +19,6 @@ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA * or visit www.oracle.com if you need additional information or have any * questions. - * */ /** diff --git a/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java b/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java index b05b10e9c1973..22b6324f24a89 100644 --- a/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java +++ b/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java @@ -19,7 +19,6 @@ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA * or visit www.oracle.com if you need additional information or have any * questions. - * */ /** From c67cedeaae68371d6489aa90af6fe53e7941d367 Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 14:00:29 +0000 Subject: [PATCH 04/13] Fix whitespace error --- src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp index e896639730e25..63d3324476b95 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp @@ -56,7 +56,7 @@ inline void exec_spin_wait_inst(SpinWait::Inst inst_id) { // } asm volatile( " tbz %[id], 0, 0f \n" // The default instruction for SpinWait is YIELD. - // We check it first before going to switch. + // We check it first before going to switch. " yield \n" " b 4f \n" "0: \n" From b2680c92da1ca92af02bb9ba0fc9b611e889a8ea Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 14:10:29 +0000 Subject: [PATCH 05/13] Clarify assert message --- src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp index 63d3324476b95..ebc18c705efd0 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp @@ -35,7 +35,7 @@ inline void exec_spin_wait_inst(SpinWait::Inst inst_id) { assert(SpinWait::ISB == 2, "SpinWait::Inst value 2 reserved for 'isb' instruction"); assert(SpinWait::SB == 4, "SpinWait::Inst value 4 reserved for 'sb' instruction"); assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); - assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or use only one bit"); + assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or power of 2"); assert(inst_id <= SpinWait::NOP, "Unsupported type of SpinWait::Inst: %d", inst_id); assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); From 629ab69b0f37fabeecea44369367595047b506b0 Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 16:02:43 +0000 Subject: [PATCH 06/13] Remove spin_wait_aarch64.inline.hpp; Combine TestSpinPauseAArch64 and TestSpinPauseSBAArch64 --- src/hotspot/cpu/aarch64/globals_aarch64.hpp | 6 +- .../cpu/aarch64/macroAssembler_aarch64.cpp | 21 +++- src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp | 1 - .../cpu/aarch64/spin_wait_aarch64.inline.hpp | 103 ------------------ .../os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 50 ++++++++- .../jtreg/gtest/TestSpinPauseAArch64.java | 12 +- .../jtreg/gtest/TestSpinPauseSBAArch64.java | 33 ------ 7 files changed, 80 insertions(+), 146 deletions(-) delete mode 100644 src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp delete mode 100644 test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java diff --git a/src/hotspot/cpu/aarch64/globals_aarch64.hpp b/src/hotspot/cpu/aarch64/globals_aarch64.hpp index 0ddccf1666489..ef741c2007adf 100644 --- a/src/hotspot/cpu/aarch64/globals_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/globals_aarch64.hpp @@ -26,7 +26,6 @@ #ifndef CPU_AARCH64_GLOBALS_AARCH64_HPP #define CPU_AARCH64_GLOBALS_AARCH64_HPP -#include "spin_wait_aarch64.hpp" #include "utilities/globalDefinitions.hpp" #include "utilities/macros.hpp" @@ -115,11 +114,10 @@ define_pd_global(intx, InlineSmallCode, 1000); "Use prfm hint with specified distance in compiled code." \ "Value -1 means off.") \ range(-1, 4096) \ - product(ccstr, OnSpinWaitInst, DEFAULT_SPIN_WAIT_INST, DIAGNOSTIC, \ + product(ccstr, OnSpinWaitInst, "yield", DIAGNOSTIC, \ "The instruction to use to implement " \ "java.lang.Thread.onSpinWait()." \ - "Options: " \ - SPIN_WAIT_INST_OPTIONS) \ + "Options: none, nop, isb, yield, sb.") \ product(uint, OnSpinWaitInstCount, 1, DIAGNOSTIC, \ "The number of OnSpinWaitInst instructions to generate." \ "It cannot be used with OnSpinWaitInst=none.") \ diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index c50b9de4a161e..6ae6861e38bb8 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -54,7 +54,6 @@ #include "runtime/jniHandles.inline.hpp" #include "runtime/sharedRuntime.hpp" #include "runtime/stubRoutines.hpp" -#include "spin_wait_aarch64.inline.hpp" #include "utilities/globalDefinitions.hpp" #include "utilities/powerOfTwo.hpp" #ifdef COMPILER1 @@ -6805,7 +6804,25 @@ void MacroAssembler::verify_cross_modify_fence_not_required() { void MacroAssembler::spin_wait() { block_comment("spin_wait {"); - generate_spin_wait(this, VM_Version::spin_wait_desc()); + for (int i = 0; i < VM_Version::spin_wait_desc().inst_count(); ++i) { + switch (VM_Version::spin_wait_desc().inst()) { + case SpinWait::NOP: + nop(); + break; + case SpinWait::ISB: + isb(); + break; + case SpinWait::YIELD: + yield(); + break; + case SpinWait::SB: + assert(VM_Version::supports_sb(), "current CPU does not support SB instruction"); + sb(); + break; + default: + ShouldNotReachHere(); + } + } block_comment("}"); } diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp index 71ce4e0689022..a4527d2977d34 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp @@ -24,7 +24,6 @@ #ifndef CPU_AARCH64_SPIN_WAIT_AARCH64_HPP #define CPU_AARCH64_SPIN_WAIT_AARCH64_HPP -#define DEFAULT_SPIN_WAIT_INST "yield" #define SPIN_WAIT_INST_OPTIONS "nop, isb, yield, sb, none" class SpinWait { diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp deleted file mode 100644 index ebc18c705efd0..0000000000000 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.inline.hpp +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -#ifndef CPU_AARCH64_SPIN_WAIT_AARCH64_INLINE_HPP -#define CPU_AARCH64_SPIN_WAIT_AARCH64_INLINE_HPP - -#include "asm/macroAssembler.hpp" -#include "runtime/vm_version.hpp" -#include "spin_wait_aarch64.hpp" -#include "utilities/powerOfTwo.hpp" - -inline void exec_spin_wait_inst(SpinWait::Inst inst_id) { - assert(SpinWait::NONE == 0, "SpinWait::Inst value 0 reserved to indicate no implementation"); - assert(SpinWait::YIELD == 1, "SpinWait::Inst value 1 reserved for 'yield' instruction"); - assert(SpinWait::ISB == 2, "SpinWait::Inst value 2 reserved for 'isb' instruction"); - assert(SpinWait::SB == 4, "SpinWait::Inst value 4 reserved for 'sb' instruction"); - assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); - assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or power of 2"); - assert(inst_id <= SpinWait::NOP, "Unsupported type of SpinWait::Inst: %d", inst_id); - assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); - - if (inst_id < SpinWait::NONE || inst_id > SpinWait::NOP) { - ShouldNotReachHere(); - } - - // The assembly code below is equivalent to the following: - // - // if (inst_id == 1) { - // exec_yield_inst(); - // } else if (inst_id == 2) { - // exec_isb_inst(); - // } else if (inst_id == 4) { - // exec_sb_inst(); - // } else if (inst_id == 8) { - // exec_nop_inst(); - // } - asm volatile( - " tbz %[id], 0, 0f \n" // The default instruction for SpinWait is YIELD. - // We check it first before going to switch. - " yield \n" - " b 4f \n" - "0: \n" - " tbnz %[id], 1, 1f \n" - " tbnz %[id], 2, 2f \n" - " tbnz %[id], 3, 3f \n" - " b 4f \n" - "1: \n" - " isb \n" - " b 4f \n" - "2: \n" - " .inst 0xd50330ff \n" // SB instruction, explicitly encoded not to rely on a compiler - " b 4f \n" - "3: \n" - " nop \n" - "4: \n" - : - : [id]"r"((uint64_t)inst_id) - : "memory"); -} - -inline void generate_spin_wait(MacroAssembler *masm, const SpinWait &spin_wait_desc) { - for (int i = 0; i < spin_wait_desc.inst_count(); ++i) { - switch (spin_wait_desc.inst()) { - case SpinWait::NOP: - masm->nop(); - break; - case SpinWait::ISB: - masm->isb(); - break; - case SpinWait::YIELD: - masm->yield(); - break; - case SpinWait::SB: - assert(VM_Version::supports_sb(), "current CPU does not support SB instruction"); - masm->sb(); - break; - default: - ShouldNotReachHere(); - } - } -} - -#endif // CPU_AARCH64_SPIN_WAIT_AARCH64_INLINE_HPP diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index 1e8d6d2dd6029..7bb04e66c0b73 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -49,8 +49,8 @@ #include "runtime/sharedRuntime.hpp" #include "runtime/stubRoutines.hpp" #include "runtime/timer.hpp" +#include "runtime/vm_version.hpp" #include "signals_posix.hpp" -#include "spin_wait_aarch64.inline.hpp" #include "utilities/align.hpp" #include "utilities/events.hpp" #include "utilities/vmError.hpp" @@ -530,7 +530,53 @@ extern "C" { // avoid a costly call to os::current_thread_enable_wx() on MacOS. // We should return 1 if SpinPause is implemented, and since there // will be always a sequence of instructions, SpinPause will always return 1. - exec_spin_wait_inst(VM_Version::spin_wait_desc().inst()); + + assert(SpinWait::NONE == 0, "SpinWait::Inst value 0 reserved to indicate no implementation"); + assert(SpinWait::YIELD == 1, "SpinWait::Inst value 1 reserved for 'yield' instruction"); + assert(SpinWait::ISB == 2, "SpinWait::Inst value 2 reserved for 'isb' instruction"); + assert(SpinWait::SB == 4, "SpinWait::Inst value 4 reserved for 'sb' instruction"); + assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); + assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or power of 2"); + assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); + + if (inst_id < SpinWait::NONE || inst_id > SpinWait::NOP) { + warining("Unsupported type of SpinWait::Inst: %d", inst_id); + ShouldNotReachHere(); + } + + // The assembly code below is equivalent to the following: + // + // if (inst_id == 1) { + // exec_yield_inst(); + // } else if (inst_id == 2) { + // exec_isb_inst(); + // } else if (inst_id == 4) { + // exec_sb_inst(); + // } else if (inst_id == 8) { + // exec_nop_inst(); + // } + asm volatile( + " tbz %[id], 0, 0f \n" // The default instruction for SpinWait is YIELD. + // We check it first before going to switch. + " yield \n" + " b 4f \n" + "0: \n" + " tbnz %[id], 1, 1f \n" + " tbnz %[id], 2, 2f \n" + " tbnz %[id], 3, 3f \n" + " b 4f \n" + "1: \n" + " isb \n" + " b 4f \n" + "2: \n" + " .inst 0xd50330ff \n" // SB instruction, explicitly encoded not to rely on a compiler + " b 4f \n" + "3: \n" + " nop \n" + "4: \n" + : + : [id]"r"((uint64_t)inst_id) + : "memory"); return 1; } diff --git a/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java b/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java index a998f6fec110b..674eb38514208 100644 --- a/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java +++ b/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java @@ -21,7 +21,7 @@ * questions. */ -/** +/* * @test TestSpinPauseAArch64 * @bug 8362193 * @summary Run SpinPause gtest using different instructions for SpinPause @@ -34,3 +34,13 @@ * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=isb * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=yield */ + +/* + * @test TestSpinPauseSBAArch64 + * @bug 8362193 + * @summary Run SpinPause gtest using SB instruction for SpinPause + * @library /test/lib + * @requires vm.flagless + * @requires (os.arch=="aarch64" & vm.cpu.features ~= ".*sb.*") + * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=sb + */ diff --git a/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java b/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java deleted file mode 100644 index 22b6324f24a89..0000000000000 --- a/test/hotspot/jtreg/gtest/TestSpinPauseSBAArch64.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -/** - * @test TestSpinPauseAArch64 - * @bug 8362193 - * @summary Run SpinPause gtest using different instructions for SpinPause - * @library /test/lib - * @requires vm.flagless - * @requires os.arch=="aarch64" - * @requires vm.cpu.features ~= ".*sb.*" - * @run main/native GTestWrapper --gtest_filter=SpinPause* -XX:+UnlockDiagnosticVMOptions -XX:OnSpinWaitInst=sb - */ From 1521ee5223cb7925be511b54882edf6a7bdfcbef Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 16:19:51 +0000 Subject: [PATCH 07/13] Fix mac compile errors --- src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index 7bb04e66c0b73..d3b0bb3a88a28 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -525,6 +525,7 @@ static inline void atomic_copy64(const volatile void *src, volatile void *dst) { } extern "C" { + // needs local assembler label '4:' to avoid trouble when using linktime optimization int SpinPause() { // We don't use StubRoutines::aarch64::spin_wait stub in order to // avoid a costly call to os::current_thread_enable_wx() on MacOS. @@ -536,10 +537,11 @@ extern "C" { assert(SpinWait::ISB == 2, "SpinWait::Inst value 2 reserved for 'isb' instruction"); assert(SpinWait::SB == 4, "SpinWait::Inst value 4 reserved for 'sb' instruction"); assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); - assert(inst_id == 0 || is_power_of_2((uint64_t)inst_id), "Values of SpinWait::Inst must be 0 or power of 2"); - assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); - if (inst_id < SpinWait::NONE || inst_id > SpinWait::NOP) { + const unit64_t inst_id = VM_Version::spin_wait_desc().inst(); + assert(inst_id == 0 || is_power_of_2(inst_id), "Values of SpinWait::Inst must be 0 or power of 2"); + assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); + if (inst_id > SpinWait::NOP) { warining("Unsupported type of SpinWait::Inst: %d", inst_id); ShouldNotReachHere(); } @@ -575,7 +577,7 @@ extern "C" { " nop \n" "4: \n" : - : [id]"r"((uint64_t)inst_id) + : [id]"r"(inst_id) : "memory"); return 1; } From c54b76cbf8958753edc62a6422abc236de54f9b3 Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 16:31:59 +0000 Subject: [PATCH 08/13] Fix mac compile errors --- src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index d3b0bb3a88a28..0b64db37e1374 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -538,7 +538,7 @@ extern "C" { assert(SpinWait::SB == 4, "SpinWait::Inst value 4 reserved for 'sb' instruction"); assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); - const unit64_t inst_id = VM_Version::spin_wait_desc().inst(); + const uint64_t inst_id = VM_Version::spin_wait_desc().inst(); assert(inst_id == 0 || is_power_of_2(inst_id), "Values of SpinWait::Inst must be 0 or power of 2"); assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); if (inst_id > SpinWait::NOP) { From 109e6676f560d280712d2ab0904f20bc307c5f1f Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 16:37:52 +0000 Subject: [PATCH 09/13] Fix mac compile errors --- src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index 0b64db37e1374..b865b738cdd00 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -52,6 +52,7 @@ #include "runtime/vm_version.hpp" #include "signals_posix.hpp" #include "utilities/align.hpp" +#include "utilities/debug.hpp" #include "utilities/events.hpp" #include "utilities/vmError.hpp" From 31cf0916ebff568d605bb6a0522435258cac97ed Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Fri, 18 Jul 2025 21:25:40 +0000 Subject: [PATCH 10/13] Update SpinWait constraints; Add test ids --- src/hotspot/cpu/aarch64/globals_aarch64.hpp | 3 ++- src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp | 2 +- src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp | 2 -- .../cpu/aarch64/vm_version_aarch64.cpp | 8 ------- .../os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 6 +---- .../flags/jvmFlagConstraintsRuntime.cpp | 22 +++++++++++++++++++ .../flags/jvmFlagConstraintsRuntime.hpp | 3 ++- .../jtreg/gtest/TestSpinPauseAArch64.java | 4 ++-- 8 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/hotspot/cpu/aarch64/globals_aarch64.hpp b/src/hotspot/cpu/aarch64/globals_aarch64.hpp index ef741c2007adf..8e520314c8b6f 100644 --- a/src/hotspot/cpu/aarch64/globals_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/globals_aarch64.hpp @@ -117,7 +117,8 @@ define_pd_global(intx, InlineSmallCode, 1000); product(ccstr, OnSpinWaitInst, "yield", DIAGNOSTIC, \ "The instruction to use to implement " \ "java.lang.Thread.onSpinWait()." \ - "Options: none, nop, isb, yield, sb.") \ + "Valid values are: none, nop, isb, yield, sb.") \ + constraint(OnSpinWaitInstNameConstraintFunc, AtParse) \ product(uint, OnSpinWaitInstCount, 1, DIAGNOSTIC, \ "The number of OnSpinWaitInst instructions to generate." \ "It cannot be used with OnSpinWaitInst=none.") \ diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp index ac926914ffd61..15b7aae683e9e 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp @@ -35,7 +35,7 @@ bool SpinWait::supports(const char *name) { } SpinWait::Inst SpinWait::from_name(const char* name) { - assert(supports(name), "spin wait instruction name must be one of: " SPIN_WAIT_INST_OPTIONS); + assert(supports(name), "checked by OnSpinWaitInstNameConstraintFunc"); if (strcmp(name, "nop") == 0) { return SpinWait::NOP; diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp index a4527d2977d34..c8c7ba7546a7d 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp @@ -24,8 +24,6 @@ #ifndef CPU_AARCH64_SPIN_WAIT_AARCH64_HPP #define CPU_AARCH64_SPIN_WAIT_AARCH64_HPP -#define SPIN_WAIT_INST_OPTIONS "nop, isb, yield, sb, none" - class SpinWait { public: // Non-zero values are chosen to have only one bit set. diff --git a/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp b/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp index f70e66ba2f7ac..9321dd0542ed8 100644 --- a/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/vm_version_aarch64.cpp @@ -51,14 +51,6 @@ uintptr_t VM_Version::_pac_mask; SpinWait VM_Version::_spin_wait; static SpinWait get_spin_wait_desc() { - if (!SpinWait::supports(OnSpinWaitInst)) { - vm_exit_during_initialization("OnSpinWaitInst is not one of " - SPIN_WAIT_INST_OPTIONS, - OnSpinWaitInst); - } - - assert(OnSpinWaitInstCount != 0, "allowed range for OnSpinWaitInstCount must not include 0"); - SpinWait spin_wait(OnSpinWaitInst, OnSpinWaitInstCount); if (spin_wait.inst() == SpinWait::SB && !VM_Version::supports_sb()) { vm_exit_during_initialization("OnSpinWaitInst is SB but current CPU does not support SB instruction"); diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index b865b738cdd00..9ebd3b56fadcf 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -52,7 +52,6 @@ #include "runtime/vm_version.hpp" #include "signals_posix.hpp" #include "utilities/align.hpp" -#include "utilities/debug.hpp" #include "utilities/events.hpp" #include "utilities/vmError.hpp" @@ -542,10 +541,7 @@ extern "C" { const uint64_t inst_id = VM_Version::spin_wait_desc().inst(); assert(inst_id == 0 || is_power_of_2(inst_id), "Values of SpinWait::Inst must be 0 or power of 2"); assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); - if (inst_id > SpinWait::NOP) { - warining("Unsupported type of SpinWait::Inst: %d", inst_id); - ShouldNotReachHere(); - } + assert(inst_id <= SpinWait::NOP, "Unsupported type of SpinWait::Inst: %llu", inst_id); // The assembly code below is equivalent to the following: // diff --git a/src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.cpp b/src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.cpp index 9e0825339c9d8..444988efdca4f 100644 --- a/src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.cpp +++ b/src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.cpp @@ -127,3 +127,25 @@ JVMFlag::Error NUMAInterleaveGranularityConstraintFunc(size_t value, bool verbos return JVMFlag::SUCCESS; } + +JVMFlag::Error OnSpinWaitInstNameConstraintFunc(ccstr value, bool verbose) { +#ifdef AARCH64 + if (value == nullptr) { + JVMFlag::printError(verbose, "OnSpinWaitInst cannot be empty\n"); + return JVMFlag::VIOLATES_CONSTRAINT; + } + + if (strcmp(value, "nop") != 0 && + strcmp(value, "isb") != 0 && + strcmp(value, "yield") != 0 && + strcmp(value, "sb") != 0 && + strcmp(value, "none") != 0) { + JVMFlag::printError(verbose, + "Unrecognized value %s for OnSpinWaitInst. Must be one of the following: " + "nop, isb, yield, sb, none\n", + value); + return JVMFlag::VIOLATES_CONSTRAINT; + } +#endif + return JVMFlag::SUCCESS; +} diff --git a/src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.hpp b/src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.hpp index 5ca28a73fb039..8425425c7681b 100644 --- a/src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.hpp +++ b/src/hotspot/share/runtime/flags/jvmFlagConstraintsRuntime.hpp @@ -40,7 +40,8 @@ f(int, ObjectAlignmentInBytesConstraintFunc) \ f(int, ContendedPaddingWidthConstraintFunc) \ f(size_t, VMPageSizeConstraintFunc) \ - f(size_t, NUMAInterleaveGranularityConstraintFunc) + f(size_t, NUMAInterleaveGranularityConstraintFunc) \ + f(ccstr, OnSpinWaitInstNameConstraintFunc) RUNTIME_CONSTRAINTS(DECLARE_CONSTRAINT) diff --git a/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java b/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java index 674eb38514208..475c86d889fd7 100644 --- a/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java +++ b/test/hotspot/jtreg/gtest/TestSpinPauseAArch64.java @@ -22,7 +22,7 @@ */ /* - * @test TestSpinPauseAArch64 + * @test id=default_armv8_0 * @bug 8362193 * @summary Run SpinPause gtest using different instructions for SpinPause * @library /test/lib @@ -36,7 +36,7 @@ */ /* - * @test TestSpinPauseSBAArch64 + * @test id=sb_armv8_5 * @bug 8362193 * @summary Run SpinPause gtest using SB instruction for SpinPause * @library /test/lib From d47fa11b392d2774eb976267b3a2746ab2075506 Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Sat, 19 Jul 2025 20:56:10 +0000 Subject: [PATCH 11/13] Add missing include --- src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp index 15b7aae683e9e..7da0151d83404 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.cpp @@ -22,6 +22,7 @@ */ #include "spin_wait_aarch64.hpp" +#include "utilities/debug.hpp" #include From e984fdeb10cc91a04860cbdbe45aacb05c4ffd3e Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Mon, 21 Jul 2025 13:42:51 +0000 Subject: [PATCH 12/13] Reimplement with switch --- src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp | 14 ++-- .../os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 66 ++++++------------- 2 files changed, 25 insertions(+), 55 deletions(-) diff --git a/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp b/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp index c8c7ba7546a7d..0e96a4b7157d4 100644 --- a/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/spin_wait_aarch64.hpp @@ -26,16 +26,12 @@ class SpinWait { public: - // Non-zero values are chosen to have only one bit set. - // This simplifies testing values in assembly code. - // This limits us to 64 possible implementation. - // Value 1 is used for the default implementation. enum Inst { - NONE = 0, - YIELD = (1 << 0), - ISB = (1 << 1), - SB = (1 << 2), - NOP = (1 << 3) + NONE = -1, + NOP, + ISB, + YIELD, + SB }; private: diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index 9ebd3b56fadcf..8fd31e6b560fa 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -52,6 +52,7 @@ #include "runtime/vm_version.hpp" #include "signals_posix.hpp" #include "utilities/align.hpp" +#include "utilities/debug.hpp" #include "utilities/events.hpp" #include "utilities/vmError.hpp" @@ -525,57 +526,30 @@ static inline void atomic_copy64(const volatile void *src, volatile void *dst) { } extern "C" { - // needs local assembler label '4:' to avoid trouble when using linktime optimization int SpinPause() { // We don't use StubRoutines::aarch64::spin_wait stub in order to // avoid a costly call to os::current_thread_enable_wx() on MacOS. // We should return 1 if SpinPause is implemented, and since there // will be always a sequence of instructions, SpinPause will always return 1. - - assert(SpinWait::NONE == 0, "SpinWait::Inst value 0 reserved to indicate no implementation"); - assert(SpinWait::YIELD == 1, "SpinWait::Inst value 1 reserved for 'yield' instruction"); - assert(SpinWait::ISB == 2, "SpinWait::Inst value 2 reserved for 'isb' instruction"); - assert(SpinWait::SB == 4, "SpinWait::Inst value 4 reserved for 'sb' instruction"); - assert(SpinWait::NOP == 8, "SpinWait::Inst value 8 reserved for 'nop' instruction"); - - const uint64_t inst_id = VM_Version::spin_wait_desc().inst(); - assert(inst_id == 0 || is_power_of_2(inst_id), "Values of SpinWait::Inst must be 0 or power of 2"); - assert(inst_id != SpinWait::SB || VM_Version::supports_sb(), "current CPU does not support SB instruction"); - assert(inst_id <= SpinWait::NOP, "Unsupported type of SpinWait::Inst: %llu", inst_id); - - // The assembly code below is equivalent to the following: - // - // if (inst_id == 1) { - // exec_yield_inst(); - // } else if (inst_id == 2) { - // exec_isb_inst(); - // } else if (inst_id == 4) { - // exec_sb_inst(); - // } else if (inst_id == 8) { - // exec_nop_inst(); - // } - asm volatile( - " tbz %[id], 0, 0f \n" // The default instruction for SpinWait is YIELD. - // We check it first before going to switch. - " yield \n" - " b 4f \n" - "0: \n" - " tbnz %[id], 1, 1f \n" - " tbnz %[id], 2, 2f \n" - " tbnz %[id], 3, 3f \n" - " b 4f \n" - "1: \n" - " isb \n" - " b 4f \n" - "2: \n" - " .inst 0xd50330ff \n" // SB instruction, explicitly encoded not to rely on a compiler - " b 4f \n" - "3: \n" - " nop \n" - "4: \n" - : - : [id]"r"(inst_id) - : "memory"); + switch (VM_Version::spin_wait_desc().inst()) { + case SpinWait::YIELD: + asm volatile("yield" : : : "memory"); + break; + case SpinWait::ISB: + asm volatile("isb" : : : "memory"); + break; + case SpinWait::SB: + assert(VM_Version::supports_sb(), "current CPU does not support SB instruction"); + asm volatile(".inst 0xd50330ff" : : : "memory"); + break; + case SpinWait::NOP: + asm volatile("nop" : : : "memory"); + break; + case SpinWait::NONE: + break; + default: + ShouldNotReachHere(); + } return 1; } From 57d30a352b66d4b1472b52c96ee824d39b71ab3b Mon Sep 17 00:00:00 2001 From: Evgeny Astigeevich Date: Mon, 21 Jul 2025 20:27:15 +0000 Subject: [PATCH 13/13] Put cases in enum order --- .../os_cpu/bsd_aarch64/os_bsd_aarch64.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp index 8fd31e6b560fa..f6e2d39e315c5 100644 --- a/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp +++ b/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp @@ -532,23 +532,25 @@ extern "C" { // We should return 1 if SpinPause is implemented, and since there // will be always a sequence of instructions, SpinPause will always return 1. switch (VM_Version::spin_wait_desc().inst()) { - case SpinWait::YIELD: - asm volatile("yield" : : : "memory"); + case SpinWait::NONE: + break; + case SpinWait::NOP: + asm volatile("nop" : : : "memory"); break; case SpinWait::ISB: asm volatile("isb" : : : "memory"); break; + case SpinWait::YIELD: + asm volatile("yield" : : : "memory"); + break; case SpinWait::SB: assert(VM_Version::supports_sb(), "current CPU does not support SB instruction"); asm volatile(".inst 0xd50330ff" : : : "memory"); break; - case SpinWait::NOP: - asm volatile("nop" : : : "memory"); - break; - case SpinWait::NONE: - break; +#ifdef ASSERT default: ShouldNotReachHere(); +#endif } return 1; }