RFR: 8221554: aarch64 cross-modifying code [v8]

Wed Nov 18 11:42:13 UTC 2020

> The AArch64 port uses maybe_isb in places where an ISB might be required
> because the code may have safepointed. These maybe_isbs are very conservative
> and are used in many places are used when a safepoint has not happened.
> 
> cross_modify_fence was added in common code to place a barrier in all the
> places after a safepoint has occurred. All the uses of it are in common code,
> yet it remains unimplemented on AArch64.
> 
> This set of patches implements cross_modify_fence for AArch64 and reconsiders
> every uses of maybe_isb, discarding many of them. In addition, it introduces
> a new diagnostic option, which when enabled on AArch64 tests the correct
> usage of the barriers.
> 
> Advantage of this patch is threefold:
> * Reducing the number of ISBs - giving a theoretical performance improvement.
> * Use of common code instead of backend specific code.
> * Additional test diagnostic options
> 
> Patch 1: Split cross_modify_fence
> =================================
> This is simply refactoring work split out to simplify the other two patches.
> 
> instruction_fence() is provided by each target and simply places
> a fence for the instruction stream.
> 
> cross_modify_fence() is now a member of JavaThread and just calls
> instruction_fence. This function will be extended in Patch 3.
> 
> Patch 2: Use cross_modify_fence instead of maybe_isb
> ====================================================
> 
> The [n] References refer to the comments for cross_modify_fence in
> thread.hpp.
> 
> This is all the existing uses of maybe_isb in the AArch64 target:
> 
> 1) Instances of Java code calling a VM function
>    * This encapsulates the changes to:
>    ** MacroAssembler::call_VM_leaf_base()
>    ** generate_fast_get_int_field0()
>    ** stubGenerator_aarch64 generate_throw_exception()
>    ** sharedRuntime_aarch64 generate_handler_blob()
>    ** SharedRuntime::generate_resolve_blob()
>    ** C1 LIR_Assembler::rt_call
>    ** C1 StubAssembler::call_RT(): used by Used by generate_exception_throw,
>                                    generate_handle_exception, generate_code_for.
>    ** OptoRuntime::generate_exception_blob()
>    * Any changes will be caught due to calls to [2] or [3] by the VM function.
>    * Any calls that do not call [2] or [3] do not require an ISB.
>    * This patch is more optimal for these cases.
> 
> 2) Instances of Java code calling a JNI function
>    * This encapsulates the changes to:
>    ** SharedRuntime::generate_native_wrapper()
>    ** TemplateInterpreterGenerator::generate_native_entry()
>    * A safepoint still in progress after the call with be caught by [4].
>    * An ISB is still required for the case where there was a safepoint
>      but it completed during the call. This happens if the code doesn't
>      branch on safepoint_in_progress
>    * In the SharedRuntime version, the two possible calls to
>      reguard_yellow_pages and complete_monitor_unlocking_C are after the thread
>      goes back into it's original state, so are covered by [2] and [3], the
>      same as a normal VM call.
>    * This patch is only more optimal for the two post-JNI calls.
> 
> 3) Patching functions
>    * This encapsulates the changes to:
>    ** patch_callers_callsite() (called by gen_c2i_adapter())
>    * This results in code being patched, but does not safepoint
>    * Therefore an ISB is required.
>    * This patch introduces no change here.
> 
> 4) C1 MacroAssembler::emit_static_call_stub()
>    * Calls ISB (not maybe_isb)
>    * By design, the patching doesn't require that the up-to-date
>      destination is required for proper functioning.
>    * However, the ISB makes it most likely that the new destination will
>      be picked up.
>    * This patch introduces no change here.
> 
> Patch 3: Add cross modify fence verification
> ============================================
> 
> The VerifyCrossModifyFence diagnostic flag enables confirmation to the correct
> usage of instruction barriers. It can safely be enabled on any Java run.
> 
> Enabling it will cause the following:
> 
> * Once all threads have been brought to a safepoint, each thread will be
>   marked.
> 
> * On a cross_modify_fence and safepoint_fence the mark for that thread
>   will be cleared.
> 
> * On entry to a method and in a safepoint poll, then the thread is checked.
>   If it is marked, then the code will error.

Alan Hayward has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:

 - Merge master 2020/11/18

   Change-Id: I256b857ca275a8806febc1b9dc5412aac6d862a7
   CustomizedGitHooks: yes
 - Enable VerifyCrossModifyFence for debug aarch64
 - Remove An instruction sync is required comments
 - Update cross_modify_fence comment
 - Update comments & remove ifdef

   Change-Id: Ibbe45650d351d8cff6fbf7a7c8baf30afbdac17c
   CustomizedGitHooks: yes
 - Merge master 2020/11/12

   Change-Id: I73323c90765bf8524f12f680abde7e7e5b3bb898
   CustomizedGitHooks: yes
 - Merge master

   Change-Id: I97df4e7686699478f0f89451ec0a3537d38cfd6d
 - Merge master

   Change-Id: I5e1715fdb11305191fe7bf86cbfb7a6da446b3dc
 - Remove inlasm_isb define

   Change-Id: I2d0ef8a78292dac875f3f65d2253981cdb7a497a
 - AArch64: Add cross modify fence verification
 - ... and 2 more: https://git.openjdk.java.net/jdk/compare/f7f34474...499d7063

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

Changes: https://git.openjdk.java.net/jdk/pull/428/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=428&range=07
  Stats: 141 lines in 25 files changed: 94 ins; 11 del; 36 mod
  Patch: https://git.openjdk.java.net/jdk/pull/428.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/428/head:pull/428

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