RFR: JDK-8287284: C2: loop optimization performs split_thru_phi infinitely many times [v3]

[Tobias Holenstein]

On Mon, 6 Nov 2023 08:56:02 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>>> `moved_to_inner_loop()` returns true if the clone may end up in an inner loop but it could return true and the clone be in the outer loop: `moved_to_inner_loop()` tests the early control but the clone will be assigned control that is as late as possible as long as it doesn't cause a node to move in a loop. In short that test is not exact. I would adjust the comment to reflect that. I also think this happens because the node that's cloned is a `Load` and anti dependence analysis is conservative and pushes it in the inner loop so we could apply the `moved_to_inner_loop()` test only to loads. Not sure if it's worth doing though.
>> 
>> So you think this problem only exists for Loads? Can we not have a similar split_thru_phi problem with another node?
>> 
>> As you say: early control is not sufficient, as it could still have all uses outside the inner loop and would then be scheduled after the inner loop (ending up in the outer loop). I guess we could just check for the late control then?
>
>> So you think this problem only exists for Loads? Can we not have a similar split_thru_phi problem with another node?
> 
> The exit of the inner loop has to be some control split with a projection that's in the outer loop. The cloned node should be assigned that projection as control (in the outer loop) unless there's something pushing it in the inner loop which I assume in this case is a store. Not sure how that would happen with other nodes.
> 
>> As you say: early control is not sufficient, as it could still have all uses outside the inner loop and would then be scheduled after the inner loop (ending up in the outer loop). I guess we could just check for the late control then?
> 
> Yes, late control would be better but it's unclear to me how straightforward it is and given this is a corner case, a simple solution is likely good enough.

I am ready to integrate. Do you agree @rwestrel ?

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

PR Comment: https://git.openjdk.org/jdk/pull/15536#issuecomment-1805408189