From c7a03cf5fcf5ed02818d509474d3430094354074 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 12 Oct 2020 13:41:50 +0100 Subject: [PATCH 01/10] Split cross_modify_fence --- .../os_cpu/aix_ppc/orderAccess_aix_ppc.hpp | 4 ++-- .../os_cpu/bsd_x86/orderAccess_bsd_x86.hpp | 4 ++-- .../os_cpu/bsd_zero/orderAccess_bsd_zero.hpp | 4 ++-- .../orderAccess_linux_aarch64.hpp | 4 ++-- .../linux_arm/orderAccess_linux_arm.hpp | 4 ++-- .../linux_ppc/orderAccess_linux_ppc.hpp | 4 ++-- .../linux_s390/orderAccess_linux_s390.hpp | 4 ++-- .../linux_x86/orderAccess_linux_x86.hpp | 4 ++-- .../linux_zero/orderAccess_linux_zero.hpp | 4 ++-- .../windows_x86/orderAccess_windows_x86.hpp | 4 ++-- src/hotspot/share/runtime/orderAccess.hpp | 24 ++++++++++++++++--- 11 files changed, 41 insertions(+), 23 deletions(-) diff --git a/src/hotspot/os_cpu/aix_ppc/orderAccess_aix_ppc.hpp b/src/hotspot/os_cpu/aix_ppc/orderAccess_aix_ppc.hpp index f866867e21cec..9ca6c18d5bb92 100644 --- a/src/hotspot/os_cpu/aix_ppc/orderAccess_aix_ppc.hpp +++ b/src/hotspot/os_cpu/aix_ppc/orderAccess_aix_ppc.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2012, 2019 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -73,7 +73,7 @@ inline void OrderAccess::storeload() { inlasm_sync(); } inline void OrderAccess::acquire() { inlasm_lwsync(); } inline void OrderAccess::release() { inlasm_lwsync(); } inline void OrderAccess::fence() { inlasm_sync(); } -inline void OrderAccess::cross_modify_fence() +inline void OrderAccess::cross_modify_fence_impl() { inlasm_isync(); } #undef inlasm_sync diff --git a/src/hotspot/os_cpu/bsd_x86/orderAccess_bsd_x86.hpp b/src/hotspot/os_cpu/bsd_x86/orderAccess_bsd_x86.hpp index 507bc1f449bcd..691f278bc950c 100644 --- a/src/hotspot/os_cpu/bsd_x86/orderAccess_bsd_x86.hpp +++ b/src/hotspot/os_cpu/bsd_x86/orderAccess_bsd_x86.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/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 @@ -59,7 +59,7 @@ inline void OrderAccess::fence() { compiler_barrier(); } -inline void OrderAccess::cross_modify_fence() { +inline void OrderAccess::cross_modify_fence_impl() { int idx = 0; __asm__ volatile ("cpuid " : "+a" (idx) : : "ebx", "ecx", "edx", "memory"); } diff --git a/src/hotspot/os_cpu/bsd_zero/orderAccess_bsd_zero.hpp b/src/hotspot/os_cpu/bsd_zero/orderAccess_bsd_zero.hpp index c168e28b6549b..be09fa24b1f60 100644 --- a/src/hotspot/os_cpu/bsd_zero/orderAccess_bsd_zero.hpp +++ b/src/hotspot/os_cpu/bsd_zero/orderAccess_bsd_zero.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright 2007, 2008, 2009 Red Hat, Inc. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -77,6 +77,6 @@ inline void OrderAccess::storeload() { FULL_MEM_BARRIER; } inline void OrderAccess::acquire() { LIGHT_MEM_BARRIER; } inline void OrderAccess::release() { LIGHT_MEM_BARRIER; } inline void OrderAccess::fence() { FULL_MEM_BARRIER; } -inline void OrderAccess::cross_modify_fence() { } +inline void OrderAccess::cross_modify_fence_impl() { } #endif // OS_CPU_BSD_ZERO_ORDERACCESS_BSD_ZERO_HPP diff --git a/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp b/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp index b07969e8d8870..bbd4c1400ffec 100644 --- a/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp +++ b/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2014, 2019, Red Hat Inc. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -53,6 +53,6 @@ inline void OrderAccess::fence() { FULL_MEM_BARRIER; } -inline void OrderAccess::cross_modify_fence() { } +inline void OrderAccess::cross_modify_fence_impl() { } #endif // OS_CPU_LINUX_AARCH64_ORDERACCESS_LINUX_AARCH64_HPP diff --git a/src/hotspot/os_cpu/linux_arm/orderAccess_linux_arm.hpp b/src/hotspot/os_cpu/linux_arm/orderAccess_linux_arm.hpp index 3bc522b17423b..3bb357704fbc0 100644 --- a/src/hotspot/os_cpu/linux_arm/orderAccess_linux_arm.hpp +++ b/src/hotspot/os_cpu/linux_arm/orderAccess_linux_arm.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 2020, Oracle and/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 @@ -101,6 +101,6 @@ inline void OrderAccess::storestore() { dmb_st(); } inline void OrderAccess::storeload() { dmb_sy(); } inline void OrderAccess::release() { dmb_sy(); } inline void OrderAccess::fence() { dmb_sy(); } -inline void OrderAccess::cross_modify_fence() { } +inline void OrderAccess::cross_modify_fence_impl() { } #endif // OS_CPU_LINUX_ARM_ORDERACCESS_LINUX_ARM_HPP diff --git a/src/hotspot/os_cpu/linux_ppc/orderAccess_linux_ppc.hpp b/src/hotspot/os_cpu/linux_ppc/orderAccess_linux_ppc.hpp index 4777ef5e60847..2e6eb1f387838 100644 --- a/src/hotspot/os_cpu/linux_ppc/orderAccess_linux_ppc.hpp +++ b/src/hotspot/os_cpu/linux_ppc/orderAccess_linux_ppc.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2012, 2014 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -77,7 +77,7 @@ inline void OrderAccess::storeload() { inlasm_sync(); } inline void OrderAccess::acquire() { inlasm_lwsync(); } inline void OrderAccess::release() { inlasm_lwsync(); } inline void OrderAccess::fence() { inlasm_sync(); } -inline void OrderAccess::cross_modify_fence() +inline void OrderAccess::cross_modify_fence_impl() { inlasm_isync(); } #undef inlasm_sync diff --git a/src/hotspot/os_cpu/linux_s390/orderAccess_linux_s390.hpp b/src/hotspot/os_cpu/linux_s390/orderAccess_linux_s390.hpp index e718e04c67c67..850433f9efa5a 100644 --- a/src/hotspot/os_cpu/linux_s390/orderAccess_linux_s390.hpp +++ b/src/hotspot/os_cpu/linux_s390/orderAccess_linux_s390.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2016, 2019 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -74,7 +74,7 @@ inline void OrderAccess::storeload() { inlasm_zarch_sync(); } inline void OrderAccess::acquire() { inlasm_zarch_acquire(); } inline void OrderAccess::release() { inlasm_zarch_release(); } inline void OrderAccess::fence() { inlasm_zarch_sync(); } -inline void OrderAccess::cross_modify_fence() { inlasm_zarch_sync(); } +inline void OrderAccess::cross_modify_fence_impl() { inlasm_zarch_sync(); } #undef inlasm_compiler_barrier #undef inlasm_zarch_sync diff --git a/src/hotspot/os_cpu/linux_x86/orderAccess_linux_x86.hpp b/src/hotspot/os_cpu/linux_x86/orderAccess_linux_x86.hpp index 444de2aa2d0f7..076cc5e8f3b45 100644 --- a/src/hotspot/os_cpu/linux_x86/orderAccess_linux_x86.hpp +++ b/src/hotspot/os_cpu/linux_x86/orderAccess_linux_x86.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/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 @@ -55,7 +55,7 @@ inline void OrderAccess::fence() { compiler_barrier(); } -inline void OrderAccess::cross_modify_fence() { +inline void OrderAccess::cross_modify_fence_impl() { int idx = 0; #ifdef AMD64 __asm__ volatile ("cpuid " : "+a" (idx) : : "ebx", "ecx", "edx", "memory"); diff --git a/src/hotspot/os_cpu/linux_zero/orderAccess_linux_zero.hpp b/src/hotspot/os_cpu/linux_zero/orderAccess_linux_zero.hpp index acd25242f0319..3c4c1de50cb52 100644 --- a/src/hotspot/os_cpu/linux_zero/orderAccess_linux_zero.hpp +++ b/src/hotspot/os_cpu/linux_zero/orderAccess_linux_zero.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright 2007, 2008, 2009 Red Hat, Inc. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -78,6 +78,6 @@ inline void OrderAccess::acquire() { LIGHT_MEM_BARRIER; } inline void OrderAccess::release() { LIGHT_MEM_BARRIER; } inline void OrderAccess::fence() { FULL_MEM_BARRIER; } -inline void OrderAccess::cross_modify_fence() { } +inline void OrderAccess::cross_modify_fence_impl() { } #endif // OS_CPU_LINUX_ZERO_ORDERACCESS_LINUX_ZERO_HPP diff --git a/src/hotspot/os_cpu/windows_x86/orderAccess_windows_x86.hpp b/src/hotspot/os_cpu/windows_x86/orderAccess_windows_x86.hpp index 60e743ddcc188..f7d990a64c47f 100644 --- a/src/hotspot/os_cpu/windows_x86/orderAccess_windows_x86.hpp +++ b/src/hotspot/os_cpu/windows_x86/orderAccess_windows_x86.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/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 @@ -58,7 +58,7 @@ inline void OrderAccess::fence() { compiler_barrier(); } -inline void OrderAccess::cross_modify_fence() { +inline void OrderAccess::cross_modify_fence_impl() { int regs[4]; __cpuid(regs, 0); } diff --git a/src/hotspot/share/runtime/orderAccess.hpp b/src/hotspot/share/runtime/orderAccess.hpp index ab522a59a8124..b8516cf84c487 100644 --- a/src/hotspot/share/runtime/orderAccess.hpp +++ b/src/hotspot/share/runtime/orderAccess.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/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 @@ -112,7 +112,7 @@ // may be more conservative in implementations. We advise using the bound // variants whenever possible. // -// Finally, we define a "fence" operation, as a bidirectional barrier. +// We define a "fence" operation, as a bidirectional barrier. // It guarantees that any memory access preceding the fence is not // reordered w.r.t. any memory accesses subsequent to the fence in program // order. This may be used to prevent sequences of loads from floating up @@ -229,6 +229,20 @@ // order*. And that their destructors do a release and unlock, in *that* // order. If their implementations change such that these assumptions // are violated, a whole lot of code will break. +// +// Finally, we define an "instruction_fence" operation, as a bidirectional +// barrier for the instruction code cache. It guarantees that any memory access +// to the instruction code preceding the fence is not reordered w.r.t. any +// memory accesses to instruction code subsequent to the fence in program order. +// It should be used in conjunction with safepointing to ensure that changes +// to the instruction stream are seen on exit from a safepoint. Namely: +// [1] Directly before running a new thread [See JavaThread::run()] +// [2] Whilst in the VM, on exit from being suspended in a safepoint. [See +// SafepointMechanism::process_if_requested_slow()] +// [3] Whilst in the VM, on exit from blocking [See ThreadBlockInVM +// and ThreadBlockInVMWithDeadlockCheck] +// [4] At the end of a JNI call, on exit from blocking. [See +// JavaThread::check_safepoint_and_suspend_for_native_trans()] class OrderAccess : public AllStatic { public: @@ -242,7 +256,9 @@ class OrderAccess : public AllStatic { static void release(); static void fence(); - static void cross_modify_fence(); + static void cross_modify_fence() { + cross_modify_fence_impl(); + } // Processors which are not multi-copy-atomic require a full fence // to enforce a globally consistent order of Independent Reads of @@ -259,6 +275,8 @@ class OrderAccess : public AllStatic { // routine if it exists, It should only be used by platforms that // don't have another way to do the inline assembly. static void StubRoutines_fence(); + + static void cross_modify_fence_impl(); }; #include OS_CPU_HEADER(orderAccess) From 853d6c70f76d83ae4f4f371d9676845edf6c3036 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 12 Oct 2020 13:41:50 +0100 Subject: [PATCH 02/10] AArch64: Use cross_modify_fence instead of maybe_isb --- .../cpu/aarch64/c1_LIRAssembler_aarch64.cpp | 4 +++- .../cpu/aarch64/c1_Runtime1_aarch64.cpp | 3 ++- .../cpu/aarch64/interp_masm_aarch64.cpp | 2 +- .../cpu/aarch64/jniFastGetField_aarch64.cpp | 3 ++- .../cpu/aarch64/macroAssembler_aarch64.cpp | 3 ++- .../cpu/aarch64/macroAssembler_aarch64.hpp | 5 ++-- .../cpu/aarch64/sharedRuntime_aarch64.cpp | 24 ++++++++++++------- .../cpu/aarch64/stubGenerator_aarch64.cpp | 3 ++- .../templateInterpreterGenerator_aarch64.cpp | 4 ++-- .../orderAccess_linux_aarch64.hpp | 8 ++++++- 10 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp index 8dac1d9ebe892..1c3ad980f89c3 100644 --- a/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp @@ -2919,10 +2919,12 @@ void LIR_Assembler::rt_call(LIR_Opr result, address dest, const LIR_OprList* arg __ blr(rscratch1); } + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. + if (info != NULL) { add_call_info_here(info); } - __ maybe_isb(); } void LIR_Assembler::volatile_move_op(LIR_Opr src, LIR_Opr dest, BasicType type, CodeEmitInfo* info) { diff --git a/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp b/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp index 2f1845affe966..04fc128171b14 100644 --- a/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp @@ -65,6 +65,8 @@ int StubAssembler::call_RT(Register oop_result1, Register metadata_result, addre // do the call lea(rscratch1, RuntimeAddress(entry)); blr(rscratch1); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. bind(retaddr); int call_offset = offset(); // verify callee-saved register @@ -80,7 +82,6 @@ int StubAssembler::call_RT(Register oop_result1, Register metadata_result, addre pop(r0, sp); #endif reset_last_Java_frame(true); - maybe_isb(); // check for pending exceptions { Label L; diff --git a/src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp b/src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp index 44b9f59bbb390..fcd8264ab1424 100644 --- a/src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp @@ -1597,7 +1597,7 @@ void InterpreterMacroAssembler::call_VM_base(Register oop_result, Label L; ldr(rscratch1, Address(rfp, frame::interpreter_frame_last_sp_offset * wordSize)); cbz(rscratch1, L); - stop("InterpreterMacroAssembler::call_VM_leaf_base:" + stop("InterpreterMacroAssembler::call_VM_base:" " last_sp != NULL"); bind(L); } diff --git a/src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp b/src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp index f1a331b592ffb..3c78f3c9bfd77 100644 --- a/src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp @@ -158,7 +158,8 @@ address JNI_FastGetField::generate_fast_get_int_field0(BasicType type) { __ enter(); __ lea(rscratch1, ExternalAddress(slow_case_addr)); __ blr(rscratch1); - __ maybe_isb(); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. __ leave(); __ ret(lr); } diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index 7eae56955694c..5dcf6508ad12f 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -1396,7 +1396,8 @@ void MacroAssembler::call_VM_leaf_base(address entry_point, bind(*retaddr); ldp(rscratch1, rmethod, Address(post(sp, 2 * wordSize))); - maybe_isb(); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. } void MacroAssembler::call_VM_leaf(address entry_point, int number_of_arguments) { diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp index 2e569ecbbd584..24980c538c362 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp @@ -1305,8 +1305,9 @@ class MacroAssembler: public Assembler { Register zlen, Register tmp1, Register tmp2, Register tmp3, Register tmp4, Register tmp5, Register tmp6, Register tmp7); void mul_add(Register out, Register in, Register offs, Register len, Register k); - // ISB may be needed because of a safepoint - void maybe_isb() { isb(); } + + // Place an ISB after code may have been modified due to a safepoint. + void safepoint_isb() { isb(); } private: // Return the effective address r + (r1 << ext) + offset. diff --git a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp index cac3e00ff9d12..f1b76fd00a3e7 100644 --- a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp @@ -373,7 +373,10 @@ static void patch_callers_callsite(MacroAssembler *masm) { __ mov(c_rarg1, lr); __ lea(rscratch1, RuntimeAddress(CAST_FROM_FN_PTR(address, SharedRuntime::fixup_callers_callsite))); __ blr(rscratch1); - __ maybe_isb(); + + // Explicit isb required because fixup_callers_callsite may change the code + // stream. + __ safepoint_isb(); __ pop_CPU_state(); // restore sp @@ -1164,7 +1167,6 @@ static void rt_call(MacroAssembler* masm, address dest) { } else { __ lea(rscratch1, RuntimeAddress(dest)); __ blr(rscratch1); - __ maybe_isb(); } } @@ -1874,7 +1876,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, __ verify_sve_vector_length(); } - // check for safepoint operation in progress and/or pending suspend requests + // Check for safepoint operation in progress and/or pending suspend requests. Label safepoint_in_progress, safepoint_in_progress_done; { // We need an acquire here to ensure that any subsequent load of the @@ -2054,6 +2056,8 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, __ str(zr, Address(rthread, in_bytes(Thread::pending_exception_offset()))); rt_call(masm, CAST_FROM_FN_PTR(address, SharedRuntime::complete_monitor_unlocking_C)); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. #ifdef ASSERT { @@ -2081,6 +2085,8 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, __ bind(reguard); save_native_result(masm, ret_type, stack_slots); rt_call(masm, CAST_FROM_FN_PTR(address, SharedRuntime::reguard_yellow_pages)); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. restore_native_result(masm, ret_type, stack_slots); // and continue __ b(reguard_done); @@ -2104,7 +2110,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, __ lea(rscratch1, RuntimeAddress(CAST_FROM_FN_PTR(address, JavaThread::check_special_condition_for_native_trans_and_transition))); } __ blr(rscratch1); - __ maybe_isb(); + // Restore any method result value restore_native_result(masm, ret_type, stack_slots); @@ -2799,6 +2805,8 @@ SafepointBlob* SharedRuntime::generate_handler_blob(address call_ptr, int poll_t __ mov(c_rarg0, rthread); __ lea(rscratch1, RuntimeAddress(call_ptr)); __ blr(rscratch1); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. __ bind(retaddr); // Set an oopmap for the call site. This oopmap will map all @@ -2812,7 +2820,6 @@ SafepointBlob* SharedRuntime::generate_handler_blob(address call_ptr, int poll_t __ reset_last_Java_frame(false); - __ maybe_isb(); __ membar(Assembler::LoadLoad | Assembler::LoadStore); if (UseSVE > 0 && save_vectors) { @@ -2910,6 +2917,8 @@ RuntimeStub* SharedRuntime::generate_resolve_blob(address destination, const cha __ lea(rscratch1, RuntimeAddress(destination)); __ blr(rscratch1); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. __ bind(retaddr); } @@ -2919,8 +2928,6 @@ RuntimeStub* SharedRuntime::generate_resolve_blob(address destination, const cha oop_maps->add_gc_map( __ offset() - start, map); - __ maybe_isb(); - // r0 contains the address we are going to jump to assuming no exception got installed // clear last_Java_sp @@ -3042,7 +3049,8 @@ void OptoRuntime::generate_exception_blob() { __ mov(c_rarg0, rthread); __ lea(rscratch1, RuntimeAddress(CAST_FROM_FN_PTR(address, OptoRuntime::handle_exception_C))); __ blr(rscratch1); - __ maybe_isb(); + // handle_exception_C is a special VM call which does not require an explicit + // instruction sync afterwards. // Set an oopmap for the call site. This oopmap will only be used if we // are unwinding the stack. Hence, all locations will be dead. diff --git a/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp b/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp index 412578eea5c6c..146dd8173853b 100644 --- a/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp @@ -5017,6 +5017,8 @@ class StubGenerator: public StubCodeGenerator { BLOCK_COMMENT("call runtime_entry"); __ mov(rscratch1, runtime_entry); __ blr(rscratch1); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. // Generate oop map OopMap* map = new OopMap(framesize, 0); @@ -5024,7 +5026,6 @@ class StubGenerator: public StubCodeGenerator { oop_maps->add_gc_map(the_pc - start, map); __ reset_last_Java_frame(true); - __ maybe_isb(); if (UseSVE > 0) { // Reinitialize the ptrue predicate register, in case the external runtime diff --git a/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp b/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp index f1a30a9ae9819..f347e0e3ed0c8 100644 --- a/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp @@ -1357,7 +1357,6 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) { // Call the native method. __ blr(r10); __ bind(native_return); - __ maybe_isb(); __ get_method(rmethod); // result potentially in r0 or v0 @@ -1410,7 +1409,8 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) { __ mov(c_rarg0, rthread); __ mov(rscratch2, CAST_FROM_FN_PTR(address, JavaThread::check_special_condition_for_native_trans)); __ blr(rscratch2); - __ maybe_isb(); + // An instruction sync is required here after the call into the VM. However, + // that will have been caught in the VM by a cross_modify_fence call. __ get_method(rmethod); __ reinit_heapbase(); __ bind(Continue); diff --git a/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp b/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp index bbd4c1400ffec..8b162a1b2ccf0 100644 --- a/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp +++ b/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp @@ -30,6 +30,8 @@ #include "runtime/vm_version.hpp" +#define inlasm_isb() asm volatile("isb" : : : "memory") + // Implementation of class OrderAccess. inline void OrderAccess::loadload() { acquire(); } @@ -53,6 +55,10 @@ inline void OrderAccess::fence() { FULL_MEM_BARRIER; } -inline void OrderAccess::cross_modify_fence_impl() { } +inline void OrderAccess::cross_modify_fence_impl() { + inlasm_isb(); +} + +#undef inlasm_isb #endif // OS_CPU_LINUX_AARCH64_ORDERACCESS_LINUX_AARCH64_HPP From 022c60e4982051bd340b81dfcc8df14222e6b71d Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 12 Oct 2020 13:41:50 +0100 Subject: [PATCH 03/10] AArch64: Add cross modify fence verification --- .../cpu/aarch64/macroAssembler_aarch64.cpp | 40 +++++++++++++++++-- .../cpu/aarch64/macroAssembler_aarch64.hpp | 7 +++- src/hotspot/share/runtime/globals.hpp | 6 ++- src/hotspot/share/runtime/orderAccess.cpp | 13 +++++- src/hotspot/share/runtime/orderAccess.hpp | 3 ++ src/hotspot/share/runtime/safepoint.cpp | 8 ++++ src/hotspot/share/runtime/thread.cpp | 9 +++++ src/hotspot/share/runtime/thread.hpp | 6 +++ 8 files changed, 85 insertions(+), 7 deletions(-) diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index 5dcf6508ad12f..0e233d8974f92 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -4403,10 +4403,15 @@ void MacroAssembler::get_polling_page(Register dest, relocInfo::relocType rtype) // Read the polling page. The address of the polling page must // already be in r. address MacroAssembler::read_polling_page(Register r, relocInfo::relocType rtype) { - InstructionMark im(this); - code_section()->relocate(inst_mark(), rtype); - ldrw(zr, Address(r, 0)); - return inst_mark(); + address mark; + { + InstructionMark im(this); + code_section()->relocate(inst_mark(), rtype); + ldrw(zr, Address(r, 0)); + mark = inst_mark(); + } + verify_cross_modify_fence_not_required(); + return mark; } void MacroAssembler::adrp(Register reg1, const Address &dest, uint64_t &byte_offset) { @@ -4471,6 +4476,7 @@ void MacroAssembler::build_frame(int framesize) { sub(sp, sp, rscratch1); } } + verify_cross_modify_fence_not_required(); } void MacroAssembler::remove_frame(int framesize) { @@ -5298,3 +5304,29 @@ void MacroAssembler::verify_ptrue() { stop("Error: the preserved predicate register (p7) elements are not all true"); bind(verify_ok); } + +void MacroAssembler::safepoint_isb() { + isb(); +#ifndef PRODUCT + if (VerifyCrossModifyFence) { + // Clear the thread state. + strb(zr, Address(rthread, in_bytes(JavaThread::requires_cross_modify_fence_offset()))); + } +#endif +} + +#ifndef PRODUCT +void MacroAssembler::verify_cross_modify_fence_not_required() { + if (VerifyCrossModifyFence) { + // Check if thread needs a cross modify fence. + ldrb(rscratch1, Address(rthread, in_bytes(JavaThread::requires_cross_modify_fence_offset()))); + Label fence_not_required; + cbz(rscratch1, fence_not_required); + // If it does then fail. + lea(rscratch1, CAST_FROM_FN_PTR(address, JavaThread::verify_cross_modify_fence_failure)); + mov(c_rarg0, rthread); + blr(rscratch1); + bind(fence_not_required); + } +} +#endif diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp index 24980c538c362..0aae3dc0fcad3 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp @@ -1307,7 +1307,7 @@ class MacroAssembler: public Assembler { void mul_add(Register out, Register in, Register offs, Register len, Register k); // Place an ISB after code may have been modified due to a safepoint. - void safepoint_isb() { isb(); } + void safepoint_isb(); private: // Return the effective address r + (r1 << ext) + offset. @@ -1383,6 +1383,11 @@ class MacroAssembler: public Assembler { } void cache_wb(Address line); void cache_wbsync(bool is_pre); + +private: + // Check the current thread doesn't need a cross modify fence. + void verify_cross_modify_fence_not_required() PRODUCT_RETURN; + }; #ifdef ASSERT diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index dfdb1b201783f..1a575b8710cbe 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -2460,7 +2460,11 @@ const intx ObjectAlignmentInBytes = 8; "Allow allocating fields in empty slots of super-classes") \ \ product(bool, DeoptimizeNMethodBarriersALot, false, DIAGNOSTIC, \ - "Make nmethod barriers deoptimise a lot.") + "Make nmethod barriers deoptimise a lot.") \ + \ + develop(bool, VerifyCrossModifyFence, false, \ + "Mark all threads after a safepoint, and clear on a modify " \ + "fence. Add cleanliness checks.") \ // end of RUNTIME_FLAGS diff --git a/src/hotspot/share/runtime/orderAccess.cpp b/src/hotspot/share/runtime/orderAccess.cpp index 96369384e969e..1b5ee7a286762 100644 --- a/src/hotspot/share/runtime/orderAccess.cpp +++ b/src/hotspot/share/runtime/orderAccess.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/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 @@ -25,7 +25,10 @@ #include "precompiled.hpp" #include "runtime/orderAccess.hpp" #include "runtime/stubRoutines.hpp" + +#ifndef PRODUCT #include "runtime/thread.hpp" +#endif void OrderAccess::StubRoutines_fence() { // Use a stub if it exists. It may not exist during bootstrap so do @@ -38,3 +41,11 @@ void OrderAccess::StubRoutines_fence() { } assert(Threads::number_of_threads() == 0, "for bootstrap only"); } + +#ifndef PRODUCT +void OrderAccess::cross_modify_fence_verify() { + if (VerifyCrossModifyFence) { + JavaThread::current()->set_requires_cross_modify_fence(false); + } +} +#endif diff --git a/src/hotspot/share/runtime/orderAccess.hpp b/src/hotspot/share/runtime/orderAccess.hpp index b8516cf84c487..983e21e7e2748 100644 --- a/src/hotspot/share/runtime/orderAccess.hpp +++ b/src/hotspot/share/runtime/orderAccess.hpp @@ -258,6 +258,7 @@ class OrderAccess : public AllStatic { static void cross_modify_fence() { cross_modify_fence_impl(); + cross_modify_fence_verify(); } // Processors which are not multi-copy-atomic require a full fence @@ -277,6 +278,8 @@ class OrderAccess : public AllStatic { static void StubRoutines_fence(); static void cross_modify_fence_impl(); + + static void cross_modify_fence_verify() PRODUCT_RETURN; }; #include OS_CPU_HEADER(orderAccess) diff --git a/src/hotspot/share/runtime/safepoint.cpp b/src/hotspot/share/runtime/safepoint.cpp index 365102fee5ea7..63ab200276e5e 100644 --- a/src/hotspot/share/runtime/safepoint.cpp +++ b/src/hotspot/share/runtime/safepoint.cpp @@ -381,6 +381,14 @@ void SafepointSynchronize::begin() { assert(_waiting_to_block == 0, "No thread should be running"); #ifndef PRODUCT + // Mark all threads + if (VerifyCrossModifyFence) { + JavaThreadIteratorWithHandle jtiwh; + for (; JavaThread *cur = jtiwh.next(); ) { + cur->set_requires_cross_modify_fence(true); + } + } + if (safepoint_limit_time != 0) { jlong current_time = os::javaTimeNanos(); if (safepoint_limit_time < current_time) { diff --git a/src/hotspot/share/runtime/thread.cpp b/src/hotspot/share/runtime/thread.cpp index e7771575c3dfe..1770a60c00df0 100644 --- a/src/hotspot/share/runtime/thread.cpp +++ b/src/hotspot/share/runtime/thread.cpp @@ -1775,6 +1775,9 @@ JavaThread::JavaThread() : ThreadSafepointState::create(this); SafepointMechanism::initialize_header(this); + + set_requires_cross_modify_fence(false); + pd_initialize(); assert(deferred_card_mark().is_empty(), "Default MemRegion ctor"); } @@ -4959,3 +4962,9 @@ void Threads::verify() { VMThread* thread = VMThread::vm_thread(); if (thread != NULL) thread->verify(); } + +#ifndef PRODUCT +void JavaThread::verify_cross_modify_fence_failure(JavaThread *thread) { + report_vm_error(__FILE__, __LINE__, "Cross modify fence failure", "%p", thread); +} +#endif diff --git a/src/hotspot/share/runtime/thread.hpp b/src/hotspot/share/runtime/thread.hpp index cb6eaa024bcb1..440ae4710f7fa 100644 --- a/src/hotspot/share/runtime/thread.hpp +++ b/src/hotspot/share/runtime/thread.hpp @@ -1109,6 +1109,7 @@ class JavaThread: public Thread { private: ThreadSafepointState* _safepoint_state; // Holds information about a thread during a safepoint address _saved_exception_pc; // Saved pc of instruction where last implicit exception happened + NOT_PRODUCT(bool _requires_cross_modify_fence;) // State used by VerifyCrossModifyFence // JavaThread termination support enum TerminatedTypes { @@ -1339,6 +1340,8 @@ class JavaThread: public Thread { SafepointMechanism::ThreadData* poll_data() { return &_poll_data; } + void set_requires_cross_modify_fence(bool val) PRODUCT_RETURN NOT_PRODUCT({ _requires_cross_modify_fence = val; }) + private: // Support for thread handshake operations HandshakeState _handshake; @@ -1634,6 +1637,7 @@ class JavaThread: public Thread { return byte_offset_of(JavaThread, _should_post_on_exceptions_flag); } static ByteSize doing_unsafe_access_offset() { return byte_offset_of(JavaThread, _doing_unsafe_access); } + NOT_PRODUCT(static ByteSize requires_cross_modify_fence_offset() { return byte_offset_of(JavaThread, _requires_cross_modify_fence); }) // Returns the jni environment for this thread JNIEnv* jni_environment() { return &_jni_environment; } @@ -1923,6 +1927,8 @@ class JavaThread: public Thread { bool is_interrupted(bool clear_interrupted); static OopStorage* thread_oop_storage(); + + static void verify_cross_modify_fence_failure(JavaThread *thread) PRODUCT_RETURN; }; // Inline implementation of JavaThread::current From 338eca42fde713cbab70cf7fd15d9fad5aab7724 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Mon, 12 Oct 2020 16:15:06 +0100 Subject: [PATCH 04/10] Remove inlasm_isb define Change-Id: I2d0ef8a78292dac875f3f65d2253981cdb7a497a --- .../os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp b/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp index 8b162a1b2ccf0..cdf314e559b8d 100644 --- a/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp +++ b/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp @@ -30,8 +30,6 @@ #include "runtime/vm_version.hpp" -#define inlasm_isb() asm volatile("isb" : : : "memory") - // Implementation of class OrderAccess. inline void OrderAccess::loadload() { acquire(); } @@ -56,7 +54,7 @@ inline void OrderAccess::fence() { } inline void OrderAccess::cross_modify_fence_impl() { - inlasm_isb(); + asm volatile("isb" : : : "memory"); } #undef inlasm_isb From d9821c57ab4205426c6c56c71116a8a0c7ab7d74 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Thu, 12 Nov 2020 11:19:25 +0000 Subject: [PATCH 05/10] Update comments & remove ifdef Change-Id: Ibbe45650d351d8cff6fbf7a7c8baf30afbdac17c CustomizedGitHooks: yes --- .../os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp | 2 -- src/hotspot/share/runtime/orderAccess.hpp | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp b/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp index cdf314e559b8d..bfae8f27ddb70 100644 --- a/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp +++ b/src/hotspot/os_cpu/linux_aarch64/orderAccess_linux_aarch64.hpp @@ -57,6 +57,4 @@ inline void OrderAccess::cross_modify_fence_impl() { asm volatile("isb" : : : "memory"); } -#undef inlasm_isb - #endif // OS_CPU_LINUX_AARCH64_ORDERACCESS_LINUX_AARCH64_HPP diff --git a/src/hotspot/share/runtime/orderAccess.hpp b/src/hotspot/share/runtime/orderAccess.hpp index 983e21e7e2748..6aafec4ee619e 100644 --- a/src/hotspot/share/runtime/orderAccess.hpp +++ b/src/hotspot/share/runtime/orderAccess.hpp @@ -230,7 +230,7 @@ // order. If their implementations change such that these assumptions // are violated, a whole lot of code will break. // -// Finally, we define an "instruction_fence" operation, as a bidirectional +// Finally, we define a "cross_modify_fence" operation, as a bidirectional // barrier for the instruction code cache. It guarantees that any memory access // to the instruction code preceding the fence is not reordered w.r.t. any // memory accesses to instruction code subsequent to the fence in program order. @@ -239,10 +239,6 @@ // [1] Directly before running a new thread [See JavaThread::run()] // [2] Whilst in the VM, on exit from being suspended in a safepoint. [See // SafepointMechanism::process_if_requested_slow()] -// [3] Whilst in the VM, on exit from blocking [See ThreadBlockInVM -// and ThreadBlockInVMWithDeadlockCheck] -// [4] At the end of a JNI call, on exit from blocking. [See -// JavaThread::check_safepoint_and_suspend_for_native_trans()] class OrderAccess : public AllStatic { public: From 7f53dd87ebd5774f6d20cb154ad83ef2a449d3ec Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 17 Nov 2020 12:15:25 +0000 Subject: [PATCH 06/10] Update cross_modify_fence comment --- src/hotspot/share/runtime/orderAccess.hpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/hotspot/share/runtime/orderAccess.hpp b/src/hotspot/share/runtime/orderAccess.hpp index 6aafec4ee619e..e2d52923c4de1 100644 --- a/src/hotspot/share/runtime/orderAccess.hpp +++ b/src/hotspot/share/runtime/orderAccess.hpp @@ -230,15 +230,9 @@ // order. If their implementations change such that these assumptions // are violated, a whole lot of code will break. // -// Finally, we define a "cross_modify_fence" operation, as a bidirectional -// barrier for the instruction code cache. It guarantees that any memory access -// to the instruction code preceding the fence is not reordered w.r.t. any -// memory accesses to instruction code subsequent to the fence in program order. -// It should be used in conjunction with safepointing to ensure that changes -// to the instruction stream are seen on exit from a safepoint. Namely: -// [1] Directly before running a new thread [See JavaThread::run()] -// [2] Whilst in the VM, on exit from being suspended in a safepoint. [See -// SafepointMechanism::process_if_requested_slow()] +// Finally, we define an "instruction_fence" operation, which ensures that all +// instructions that come after the fence in program order are fetched +// from the cache or memory after the fence has completed. class OrderAccess : public AllStatic { public: From 76abe6135ffbcf4c21526c978e036217d76b4d22 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 17 Nov 2020 12:18:54 +0000 Subject: [PATCH 07/10] Remove An instruction sync is required comments --- src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp | 3 --- src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp | 2 -- src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp | 2 -- src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp | 2 -- src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp | 8 -------- src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp | 2 -- .../cpu/aarch64/templateInterpreterGenerator_aarch64.cpp | 2 -- 7 files changed, 21 deletions(-) diff --git a/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp index 1c3ad980f89c3..b0804c48fba42 100644 --- a/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp @@ -2919,9 +2919,6 @@ void LIR_Assembler::rt_call(LIR_Opr result, address dest, const LIR_OprList* arg __ blr(rscratch1); } - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. - if (info != NULL) { add_call_info_here(info); } diff --git a/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp b/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp index 04fc128171b14..6d266aaac1f91 100644 --- a/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp @@ -65,8 +65,6 @@ int StubAssembler::call_RT(Register oop_result1, Register metadata_result, addre // do the call lea(rscratch1, RuntimeAddress(entry)); blr(rscratch1); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. bind(retaddr); int call_offset = offset(); // verify callee-saved register diff --git a/src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp b/src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp index 2b3051d275434..a0444617c4b5f 100644 --- a/src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/jniFastGetField_aarch64.cpp @@ -159,8 +159,6 @@ address JNI_FastGetField::generate_fast_get_int_field0(BasicType type) { __ enter(); __ lea(rscratch1, ExternalAddress(slow_case_addr)); __ blr(rscratch1); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. __ leave(); __ ret(lr); } diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index 842c797b41cbc..efb8d702a9ac5 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -1381,8 +1381,6 @@ void MacroAssembler::call_VM_leaf_base(address entry_point, bind(*retaddr); ldp(rscratch1, rmethod, Address(post(sp, 2 * wordSize))); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. } void MacroAssembler::call_VM_leaf(address entry_point, int number_of_arguments) { diff --git a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp index 4cbb098d6989d..cf130aca67691 100644 --- a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp @@ -2038,8 +2038,6 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, __ str(zr, Address(rthread, in_bytes(Thread::pending_exception_offset()))); rt_call(masm, CAST_FROM_FN_PTR(address, SharedRuntime::complete_monitor_unlocking_C)); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. #ifdef ASSERT { @@ -2067,8 +2065,6 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, __ bind(reguard); save_native_result(masm, ret_type, stack_slots); rt_call(masm, CAST_FROM_FN_PTR(address, SharedRuntime::reguard_yellow_pages)); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. restore_native_result(masm, ret_type, stack_slots); // and continue __ b(reguard_done); @@ -2772,8 +2768,6 @@ SafepointBlob* SharedRuntime::generate_handler_blob(address call_ptr, int poll_t __ mov(c_rarg0, rthread); __ lea(rscratch1, RuntimeAddress(call_ptr)); __ blr(rscratch1); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. __ bind(retaddr); // Set an oopmap for the call site. This oopmap will map all @@ -2884,8 +2878,6 @@ RuntimeStub* SharedRuntime::generate_resolve_blob(address destination, const cha __ lea(rscratch1, RuntimeAddress(destination)); __ blr(rscratch1); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. __ bind(retaddr); } diff --git a/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp b/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp index 71aa62b9f2e38..e7dc323b0451d 100644 --- a/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp @@ -5622,8 +5622,6 @@ class StubGenerator: public StubCodeGenerator { BLOCK_COMMENT("call runtime_entry"); __ mov(rscratch1, runtime_entry); __ blr(rscratch1); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. // Generate oop map OopMap* map = new OopMap(framesize, 0); diff --git a/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp b/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp index d8f8633d14e81..c73b42579205f 100644 --- a/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp @@ -1409,8 +1409,6 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) { __ mov(c_rarg0, rthread); __ mov(rscratch2, CAST_FROM_FN_PTR(address, JavaThread::check_special_condition_for_native_trans)); __ blr(rscratch2); - // An instruction sync is required here after the call into the VM. However, - // that will have been caught in the VM by a cross_modify_fence call. __ get_method(rmethod); __ reinit_heapbase(); __ bind(Continue); From 3d68fdeb545ebdf1928a365f1997d341f4d9a89b Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Tue, 17 Nov 2020 12:19:21 +0000 Subject: [PATCH 08/10] Enable VerifyCrossModifyFence for debug aarch64 --- src/hotspot/share/runtime/globals.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index 9a3d511c937fe..859c2da97e4c9 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -2507,7 +2507,8 @@ const intx ObjectAlignmentInBytes = 8; product(bool, DeoptimizeNMethodBarriersALot, false, DIAGNOSTIC, \ "Make nmethod barriers deoptimise a lot.") \ \ - develop(bool, VerifyCrossModifyFence, false, \ + develop(bool, VerifyCrossModifyFence, \ + false AARCH64_ONLY(DEBUG_ONLY(||true)), \ "Mark all threads after a safepoint, and clear on a modify " \ "fence. Add cleanliness checks.") \ From 56429ca8ec4b418414968f4178c518da806b6d07 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Wed, 18 Nov 2020 11:56:19 +0000 Subject: [PATCH 09/10] Fix global flags indentation --- src/hotspot/share/runtime/globals.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index 311a55263ca4c..fc2fd82733311 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -2493,7 +2493,7 @@ const intx ObjectAlignmentInBytes = 8; "Make nmethod barriers deoptimise a lot.") \ \ develop(bool, VerifyCrossModifyFence, \ - false AARCH64_ONLY(DEBUG_ONLY(||true)), \ + false AARCH64_ONLY(DEBUG_ONLY(||true)), \ "Mark all threads after a safepoint, and clear on a modify " \ "fence. Add cleanliness checks.") \ From 655497a09538bb7d3e008d71cdacf6a181660d23 Mon Sep 17 00:00:00 2001 From: Alan Hayward Date: Wed, 18 Nov 2020 17:12:14 +0000 Subject: [PATCH 10/10] Add cross_modify_fence_impl for Windows AArch64 Change-Id: I8701ea60d2823d16666cb43cb9d0935d92b81e52 CustomizedGitHooks: yes --- .../os_cpu/windows_aarch64/orderAccess_windows_aarch64.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/hotspot/os_cpu/windows_aarch64/orderAccess_windows_aarch64.hpp b/src/hotspot/os_cpu/windows_aarch64/orderAccess_windows_aarch64.hpp index 11ec7322a9f42..5385f3e6a1028 100644 --- a/src/hotspot/os_cpu/windows_aarch64/orderAccess_windows_aarch64.hpp +++ b/src/hotspot/os_cpu/windows_aarch64/orderAccess_windows_aarch64.hpp @@ -28,7 +28,7 @@ // Included in orderAccess.hpp header file. #include using std::atomic_thread_fence; -#include +#include #include "vm_version_aarch64.hpp" #include "runtime/vm_version.hpp" @@ -55,6 +55,8 @@ inline void OrderAccess::fence() { FULL_MEM_BARRIER; } -inline void OrderAccess::cross_modify_fence() { } +inline void OrderAccess::cross_modify_fence_impl() { + __isb(_ARM64_BARRIER_SY); +} #endif // OS_CPU_WINDOWS_AARCH64_ORDERACCESS_WINDOWS_AARCH64_HPP