RFR: 8360031: C2 compilation asserts in MemBarNode::remove

[Aleksey Shipilev]

On Wed, 30 Jul 2025 15:08:29 GMT, Damon Fenacci <dfenacci at openjdk.org> wrote:

> # Issue
> While compiling `java.util.zip.ZipFile` in C2 this assert is triggered
> https://github.com/openjdk/jdk/blob/a2e86ff3c56209a14c6e9730781eecd12c81d170/src/hotspot/share/opto/memnode.cpp#L4235
> 
> # Cause
> While compiling the constructor of java.util.zip.ZipFile$CleanableResource the following happens:
> * we insert a trailing `MemBarStoreStore` in the constructor
> <img height="200" alt="before_folding" src="https://github.com/user-attachments/assets/c1aab634-808d-4198-94ac-8093c6b85c5d" />
> 
> * during IGVN we completely fold the memory subtree of the `MemBarStoreStore` node. The node still has a control output attached. 
> <img height="200" alt="after_folding" src="https://github.com/user-attachments/assets/568e9fc3-5f19-4e10-a72e-f0a5e772daed" />
> 
> * later during the same IGVN run the `MemBarStoreStore` node is handled and we try to remove it (because the `Allocate` node of the `MembBar` is not escaping the thread ) https://github.com/openjdk/jdk/blob/7b7136b4eca15693cfcd46ae63d644efc8a88d2c/src/hotspot/share/opto/memnode.cpp#L4301-L4302
> * the assert https://github.com/openjdk/jdk/blob/7b7136b4eca15693cfcd46ae63d644efc8a88d2c/src/hotspot/share/opto/memnode.cpp#L4235
> triggers because the barrier has only 1 (control) output and is a `MemBarStoreStore` (not `Initialize`) barrier
> 
> The issue happens only when the `UseStoreStoreForCtor` is set (default as well), which makes C2 use `MemBarStoreStore` instead of `MemBarRelease` at the end of constructors. `MemBarStoreStore` are processed separately by EA and this happens after the IGVN pass that folds the memory subtree. `MemBarRelease` on the other hand are handled during same IGVN pass before the memory subtree gets removed and it’s still got 2 outputs (assert skipped).
> 
> # Fix
> Adapting the assert to accept that `MemBarStoreStore` can also have `!= 2` outputs (when `+UseStoreStoreForCtor` is used) seems to be an OK solution as this seems like a perfectly plausible situation.
> 
> # Testing
> Unfortunately reproducing the issue with a simple regression test has proven very hard. The test seems to rely on very peculiar profiling and IGVN worklist sequence. JBS replay compilation passes. Running JCK's `api/java_util` 100 times triggers the assert a couple of times on average before the fix, none after.
> Tier 1-3+ tests passed.

This looks reasonable to me. So it looks to be an overly zealous assert rather than compiler bug? Someone more savvy with C2 code need to look and confirm.

Oh, maybe pull from the recent master to get GHA fixes, and other fixes?

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26556#pullrequestreview-3119874496
PR Comment: https://git.openjdk.org/jdk/pull/26556#issuecomment-3187809417