RFR: 8334060: Implementation of Late Barrier Expansion for G1 [v9]

[Kim Barrett]

On Mon, 19 Aug 2024 08:53:30 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> This changeset implements JEP 475 (Late Barrier Expansion for G1), including support for the x64 and aarch64 platforms. See the [JEP description](https://openjdk.org/jeps/475) for further detail.
>> 
>> We aim to integrate this work in JDK 24. The purpose of this pull request is double-fold:
>> 
>> - to allow maintainers of the arm (32-bit), ppc, riscv, s390, and x86 (32-bit) ports to contribute a port of these platforms in time for JDK 24; and
>> - to allow reviewers to review the platform-independent, x64 and aarch64, and test changes in parallel with the porting work.
>> 
>> ## Summary of the Changes
>> 
>> ### Platform-Independent Changes (`src/hotspot/share`)
>> 
>> These consist mainly of:
>> 
>> - a complete rewrite of `G1BarrierSetC2`, to instruct C2 to expand G1 barriers late instead of early;
>> - a few minor changes to C2 itself, to support removal of redundant decompression operations and to address an OopMap construction issue triggered by this JEP's increased usage of ADL `TEMP` operands; and
>> - temporary support for porting the JEP to the remaining platforms.
>> 
>> The temporary support code (guarded by the pre-processor flag `G1_LATE_BARRIER_MIGRATION_SUPPORT`) will **not** be part of the final pull request, and hence does not need to be reviewed.
>> 
>> ### Platform-Dependent Changes (`src/hotspot/cpu`)
>> 
>> These include changes to the ADL instruction definitions and the `G1BarrierSetAssembler` class of the x64 and aarch64 platforms.
>> 
>> #### ADL Changes
>> 
>> The changeset uses ADL predicates to force C2 to implement memory accesses tagged with barrier information using G1-specific, barrier-aware instruction versions (e.g. `g1StoreP` instead of the GC-agnostic `storeP`). These new instruction versions generate machine code accordingly to the corresponding tagged barrier information, relying on the G1 barrier implementations provided by the `G1BarrierSetAssembler` class. In the aarch64 platform, the bulk of the ADL code is generated from a higher-level version using m4, to reduce redundancy.
>> 
>> #### `G1BarrierSetAssembler` Changes
>> 
>> Both platforms basically reuse the barrier implementation for the bytecode interpreter, with the different barrier tests and operations refactored into dedicated functions. Besides this, `G1BarrierSetAssembler` is extended with assembly-stub routines that implement the out-of-line, slow path of the barriers. These routines include calls from the barrier into the JVM, which require support for saving and restoring live ...
>
> Roberto Castañeda Lozano has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Pass oldval to the pre-barrier in g1CompareAndExchange/SwapP

src/hotspot/cpu/aarch64/gc/g1/g1BarrierSetAssembler_aarch64.cpp line 218:

> 216:     __ cbz(new_val, done);
> 217:   }
> 218:   // Storing region crossing non-null, is card already dirty?

s/already dirty/young/

src/hotspot/cpu/aarch64/gc/g1/g1BarrierSetAssembler_aarch64.cpp line 280:

> 278: 
> 279: #undef __
> 280: #define __ masm->

These "changes" to `__` are unnecessary and confusing.  We have the same define near the top of
the file, unconditionally.  This one is conditonal on COMPILER2, but is left in place at the end of the
conditional block, affecting following unconditional code.

src/hotspot/share/opto/memnode.cpp line 3468:

> 3466:   // Capture an unaliased, unconditional, simple store into an initializer.
> 3467:   // Or, if it is independent of the allocation, hoist it above the allocation.
> 3468:   if (ReduceFieldZeroing && ReduceInitialCardMarks && /*can_reshape &&*/

It's not obvious to me how this is related to the late barrier changes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19746#discussion_r1730194278
PR Review Comment: https://git.openjdk.org/jdk/pull/19746#discussion_r1730238757
PR Review Comment: https://git.openjdk.org/jdk/pull/19746#discussion_r1730246320