RFR: 8364757: Missing Store nodes caused by bad wiring in PhaseIdealLoop::insert_post_loop

[Emanuel Peter]

On Tue, 16 Sep 2025 08:34:48 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/loopTransform.cpp line 1692:
>> 
>>> 1690:   }
>>> 1691:   return out;
>>> 1692: }
>> 
>> Note from later me: I was quite confused here. I thought this was going to be some general function that should handle all sorts of memory flow in the loop, but that is not the case. I'll leave all my comments here just to show you what I as the reader thought when reading it ;)
>> 
>> Below, in a code comment you say that this method does:
>> `Find the last memory node in the loop when following memory usages`
>> 
>> What happens here if we hit an if-diamond (or more complicated), where there can be multiple memory uses, that are then merged again by a memory phi?
>> 
>> 
>> store
>>  |
>>  +--------+
>>  |        |
>> store   store
>>  |        |
>>  +---+ +--+
>>      | |
>>      phi
>>       |
>>     store -> the last one in the loop
>> 
>> I wonder if this is somehow possible. There are surely some IGVN optimizations that would common the stores here, and so the graph would probably have to be even more complicated. But I'm simply wondering if it could be possible that we would have branches / phis in the memory graph. Or what guarantees us that the graph is really linear here?
>> 
>> I'm also not sure how to parse the method name:
>> `find_mem_out_outer_strip_mined`
>> - find "mem out" <something> outer-strip-mined <loop?>
>> - find mem outside of outer-strip-mined loop?
>
> I suppose we would trigger your assert if we found a branch:
> `assert(unique_next == nullptr, "memory node should only have one usage in the loop body");`
> 
> Now we usually only do pre-main-post for relatively small loop bodies, see `LoopUnrollLimit`. But I wonder if we ever decided to increase this limit, would we then encounter such more complicated memory graphs?

Ok, I think I have been misled by the names / comments.
You are really looking for the last store in the `outer_loop`. And we do have the guarantee of a linear memory graph because it is the one between `if_false` and SafePoint.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27225#discussion_r2351568383