RFR: 8356708: C2: loop strip mining expansion doesn't take sunk stores into account [v6]

[Emanuel Peter]

On Wed, 18 Jun 2025 06:38:05 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Roland Westrelin has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - review
>>  - more
>
> src/hotspot/share/opto/loopnode.cpp line 338:
> 
>> 336: // to be mostly unaffected by the outer strip mined loop: the only extra step needed in most cases is to step over the
>> 337: // OuterStripMinedLoop. The main drawback is that once loop optimizations are over, an extra step is needed to finish
>> 338: // constructing the outer loop. This is handled by OuterStripMinedLoopNode::adjust_strip_mined_loop().
> 
> You should probably say explicitly which C2 IR rule we violate: whenever a memory slice is mutated in a loop, there needs to be a corresponding phi.

Suggestion:

// Not adding Phis to the outer loop head from the beginning, and only adding them after loop optimizations
// does not conform to C2's IR rules: any variable or memory slice that is mutated in a loop should have a Phi.
// The main motivation for such a design that doesn't conform to C2's IR rules is to allow existing loop optimizations
// to be mostly unaffected by the outer strip mined loop: the only extra step needed in most cases is to step over the
// OuterStripMinedLoop. The main drawback is that once loop optimizations are over, an extra step is needed to finish
// constructing the outer loop. This is handled by OuterStripMinedLoopNode::adjust_strip_mined_loop().

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25717#discussion_r2153793259