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

[Vladimir Kozlov]

On Mon, 9 Sep 2024 14:41:25 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/c2/g1BarrierSetC2.cpp line 112:
>> 
>>> 110:           // The answer is that stores of different sizes can co-exist
>>> 111:           // in the same sequence of RawMem effects.  We sometimes initialize
>>> 112:           // a whole 'tile' of array elements with a single jint or jlong.)
>> 
>> I'm having trouble making sense of this comment.  I guess a jlong could be used to null-initialize two
>> 32bit oops/narrowOops?  But that doesn't have anything to do with jints.
>
> I am not sure the complex overlap test is necessary here, this code was copy-pasted from [MemNode::find_previous_store()](https://github.com/openjdk/jdk/blob/c54fc08aa3c63e4b26dc5edb2436844dfd3bab7c/src/hotspot/share/opto/memnode.cpp#L678) by [JDK-8057737](https://bugs.openjdk.org/browse/JDK-8057737), and in this new context I do not see how we might find stores of different sizes as mentioned in the comment. jlongs could be used to null-initialize two 32-bit OOPs, but such initializing stores are not even visible in C2's intermediate representation at the time `G1BarrierSetC2::g1_can_remove_pre_barrier()` is called. The fact that the comment refers to initializing several array elements with a single jint suggests to me that this code has lost some of its original purpose after being copied into a narrower context (OOP stores after object allocations). But since this code is pre-existing and in the worst case it is just performing some unnecessary work, I suggest to leave it as-is
  and possibly investigate how to simplify it as a follow-up task.

Yes, the comment reference to combined initialization stores: [memnode.cpp#L4925](https://github.com/openjdk/jdk/blob/c54fc08aa3c63e4b26dc5edb2436844dfd3bab7c/src/hotspot/share/opto/memnode.cpp#L4925)
Which is used only for primitive type (integers and floats) constant strores.
There was also recent change by Emanuel to combine stores into primitive arrays: [JDK-8335390](https://bugs.openjdk.org/browse/JDK-8335390)

None of above do anything to oop stores.  I agree that this code could left for now and be optimized later.

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

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