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

[Hamlin Li]

On Wed, 2 Oct 2024 11:40:18 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

> If `res` was not marked as `dont_preserve`, it would be included in the pre-barrier stub's preserve set (`BarrierStubC2::preserve_set()`) because it is live out of the entire AD instruction (as computed by `BarrierSetC2::compute_liveness_at_stubs()`).

Thanks for explanation!
I did not realize this, if that's the case, then it's good.

>> src/hotspot/cpu/riscv/gc/g1/g1_riscv.ad line 169:
>> 
>>> 167:   predicate(UseG1GC && n->as_LoadStore()->barrier_data() != 0);
>>> 168:   match(Set res (CompareAndExchangeP mem (Binary oldval newval)));
>>> 169:   effect(TEMP res, TEMP tmp1, TEMP tmp2);
>> 
>> should `res` be `TEMP_DEF`?
>
> It could, but the effect would be the same (see [JDK-8058880](https://bugs.openjdk.org/browse/JDK-8058880)). I went with `TEMP` for the x64 and aarch64 platforms for consistency with the analogous ZGC ADL code, see e.g. https://github.com/openjdk/jdk/blob/855c8a7def21025bc2fc47594f7285a55924c213/src/hotspot/cpu/aarch64/gc/z/z_aarch64.ad#L182-L204.

I saw the riscv one in z_riscv.ad is: `effect(TEMP oldval_tmp, TEMP newval_tmp, TEMP tmp1, TEMP_DEF res);`, maybe it's good  to change riscv one?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19746#discussion_r1784479784
PR Review Comment: https://git.openjdk.org/jdk/pull/19746#discussion_r1784479526