RFR: 8327963: C2: fix construction of memory graph around Initialize node to prevent incorrect execution if allocation is removed [v8]

[John R Rose]

On Thu, 12 Jun 2025 15:39:35 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> I think it would be good (although not necessarily in the context of this PR) to establish the "no duplicate memory projection" invariant in the back-end, for sanity and to make sure we do not break any logic that might be implicitly relying on it. If you agree, could you file a follow-up RFE, ideally with a reproducer where the current logic fails to remove `NarrowMemProj`s?

I see this as a request for a better "normal form" for the graph.  The trick here is that, if we are allowing temporary "abnormal" forms of the graph, in order to give various transforms some "working room" to rearrange things, we need to decide when are the moments when the graph must be settled back down into a normal form.

We sometimes check for some kinds of IR normality, and/or enforce some normality, in the "final graph reshape" phase.  The problem with loading up too many ad hoc operations at that point is, it may create a completely new kind of graph with new invariants.  (Don't like the current standard?  Create a new one, and see how that goes!  Same for global IR contracts.)  

Having two kinds of IR with two sets of invariants (one set more restrictive) has an obvious objection:  We fragment our ability to enforce the rules; we need to write enforcement logic which says "which phase are we in?" before checking the right set of rules.  And if the editing sessions are rare, we don't get much benefit from the rules that are enforced by that editing session.  By definition "final graph reshape" is rare.  It's worth it since we are going to a lower IR, which really must have different rules, but it's not a light thing to add to the design.

In any case, adding a normalization requirement seems to need a "wash pass" of some sort over the whole graph, to do necessary cleanups.  We do this sometimes, I think, after loop opts or EA, maybe other places, and at "final graph reshape".  This is going to be a runtime expense, I think, unless it can be piggybacked on some other pass we already do.  Maybe a hallmark of these "post-operative" cleanups is that the operation itself required some side data structure, created just for the operation (loop nest or connection graph) and discarded later in order to unleash unconstrained downstream transforms.  During the operation, transforms are specialized just to keep the side data structure relevant.  Afterwards, the graph "opens up" to unconstrained changes.  But in all cases, local updates should be as free as possible, even if their order varies randomly due to worklist artifacts, etc.  (BTW, this is why stress tests on worklist order are valuable.)

I'm not advocating firmly for or against new normalizations, but here's a final thought to throw in:  Performing normalizations seems to distrust an important design decision, noted in my previous comment.  That is, IR transformations should a "confluent" or "commutative"; it should not matter in what order you perform the transforms; you should still get to a better program, with identical user-visible semantics, whichever order you apply the transforms.  (Worklist stress tests, again…)  Obviously we globally schedule our tactics at the top level, but (in a deep sense) it should rarely matter what order we schedule things, at least as far as correctness goes.  And down in the details, at a local level, it really should not matter what form of graph you are working with.  Specifically, if we are using narrow memory projections sometimes, we should be prepared to respect them always. (Except, perhaps, at very well defined global cut-points, like final graph reshape, or a comprehensi
 ve cleanup after episodes of loop opts or EA.)

It's been a while since I coded C2 stuff as my day job, but HTH.

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

PR Comment: https://git.openjdk.org/jdk/pull/24570#issuecomment-3063284364