RFR: 8327963: C2: fix construction of memory graph around Initialize node to prevent incorrect execution if allocation is removed [v8]
Roland Westrelin
roland at openjdk.org
Tue Sep 9 11:30:13 UTC 2025
On Fri, 11 Jul 2025 18:20:19 GMT, John R Rose <jrose 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?
>>
>> One way would be to simply assert that there's no `NarrowMemProj`s left during final graph reshape. Is that what you'd like?
>> Stepping back, what's the concern here? The new projections should mostly be harmless.
>
>> 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 ord...
@rose00 @robcasloz I updated the change with a new way to avoid redundant projections. At matching time, before a `NarrowMemProj` is matched into a `MachProj`, new logic checks whether a `MachProj` already exists. That guarantees that no redundant `MachProj` are ever added. It also performs the new normalization at a major cut-point. What do you think?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/24570#issuecomment-3270256703
More information about the hotspot-compiler-dev
mailing list