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

[Roberto Castañeda Lozano]

On Tue, 13 Aug 2024 20:42:36 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> Good question! I have checked (no pun intended) and it turns out C2 never uses stores (or other memory access operations) with late-expanded barriers to perform implicit null checks. This is accidental, due to the fact that all these memory operations use MachTemp nodes in the C2 code generation stage (to reserve registers for their ADL TEMP operands). C2's implicit null check analysis requires that all inputs of a candidate memory operation dominate the null check [1], which fails if the operation uses MachTemp nodes, since these are always placed in the same basic block [2].
>> 
>> Note that this optimization triggers very rarely, if at all, for memory operations in the current early barrier expansion model, since the additional control flow of the barrier code obfuscates the analysis. For late barrier expansion, the analysis could be easily extended to recognize and hoist MachTemp nodes together with their user memory operation that is a candidate to implement the implicit null check [3], but that would require extending the barrier assembly emission step to populate the implicit null exception table correctly. Since this seems non-trivial, and would also affect other garbage collectors (ZGC), I suggest to simply assert for now that we do not generate implicit null checks for memory operations with barrier data (as in [4]), and leave full support for implicit null checks for these G1 and ZGC operations to a future RFE. What do you think?
>> 
>> [1] https://github.com/robcasloz/jdk/blob/d21104ca8ff1eef88a9d87fb78dda3009414b5b8/src/hotspot/share/opto/lcm.cpp#L310-L328
>> [2] https://github.com/robcasloz/jdk/blob/d21104ca8ff1eef88a9d87fb78dda3009414b5b8/src/hotspot/share/opto/gcm.cpp#L1397-L1404
>> [3] https://github.com/robcasloz/jdk/commit/e0ab1e418b81c0acddff2190ca57b3335b5214ba
>> [4] https://github.com/robcasloz/jdk/commit/e0ab1e418b81c0acddff2190ca57b3335b5214ba#diff-554fddca91406a67dc0f8faee12dc30c709181685a0add7f4ba9ae5ace68f192R2031-R2032
>
> Thanks for figuring it out! Makes sense.

Added the assertion in commit 554de779.

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

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