RFR: 8336702: C2 compilation fails with "all memory state should have been processed" assert

[Emanuel Peter]

On Mon, 16 Sep 2024 08:34:44 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> When converting a `LongCountedLoop` into a loop nest, c2 needs jvm
> state to add predicates to the inner loop. For that, it peels an
> iteration of the loop and uses the state of the safepoint at the end
> of the loop. That's only legal if there's no side effect between the
> safepoint and the backedge that goes back into the loop. The assert
> failure here happens in code that checks that.
> 
> That code compares the memory states at the safepoint and at the
> backedge. If they are the same then there's no side effect. To check
> consistency, the `MergeMem` at the safepoint is cloned. As the logic
> iterates over the backedge state, it clears every component of the
> state it encounters from the `MergeMem`. Once done, the cloned
> `MergeMem` should be "empty". In the case of this failure, no side
> effect is found but the cloned `MergeMem` is not empty. That happens
> because of EA: it adds edges to the `MergeMem` at the safepoint that
> it doesn't add to the backedge `Phis`.
> 
> So it's the verification code that fails and I propose dealing with
> this by ignoring memory state added by EA in the verification code.

Drive-by comment. I don't understand enough about EA to review the VM part.

test/hotspot/jtreg/compiler/longcountedloops/TestSafePointWithEAState.java line 59:

> 57:   float n;
> 58:   h(float n) { this.n = n; }
> 59: }

Java indentation is supposed to be 4 spaces ;)
Adding some explicit brackets would also be nice, but that is more subjective.

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21009#pullrequestreview-2312486543
PR Review Comment: https://git.openjdk.org/jdk/pull/21009#discussion_r1764934007