RFR: 8335747: C2: fix overflow case for LoopLimit with constant inputs

[Quan Anh Mai]

On Fri, 10 Jan 2025 06:20:08 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> `LoopLimitNode::Value` tries to constant-fold when it has constant inputs. However, there can be an overflow in the int-computation, but we check for it with `if (final_con == (jlong)final_int) {` and do not constant fold in that case.
> 
> However, there was an `assert` that checked that such an overflow would never be encountered. We already had to make an exception for this assert during PhaseCCP with [JDK-8309266](https://bugs.openjdk.org/browse/JDK-8309266).
> 
> Why did we not hit this assert before?
> `LoopLimitNode` needs to have constant inputs. We used to assume that if the constants would lead to an overflow, then the loop-limit-check would also get similar constants, and detect that `limit <= max_int-stride` does not hold, and it would constant-fold away the loop, together with the `LoopLimitNode`.
> 
> But now we found a second case:
> https://github.com/openjdk/jdk/blob/d93d1a3b58728f7978bbd5824b1bf6493b42561e/src/hotspot/share/opto/loopnode.cpp#L2555-L2563
> 
> In `PhaseIdealLoop::split_thru_phi`, we temporarily split the `LoopLimitNode` through the phi, generating a new `LoopLimitNode` for each branch of the `phi`. We then call `Value` on it to see if that leads us to constant fold one of the branches, which would be considered a "win".
> https://github.com/openjdk/jdk/blob/d93d1a3b58728f7978bbd5824b1bf6493b42561e/src/hotspot/share/opto/loopopts.cpp#L87-L105
> 
> In the regression test, we have this example:
> https://github.com/openjdk/jdk/blob/d93d1a3b58728f7978bbd5824b1bf6493b42561e/test/hotspot/jtreg/compiler/loopopts/TestLoopLimitOverflowDuringSplitThruPhi.java#L44-L69
> 
> We generate a temporary clone of `LoopLimitNode(init=0, limit=x, stride=4)` (would not constant fold because of variable `x = Phi(1000, 2147483647)`), which happens to be `LoopLimitNode(init=0, limit=2147483647, stride=4)`. We evaluate `Value` on this temporary clone, and hit the overflow case.
> 
> Why is it ok to just remove the assert and allow `LoopLimitNode` to overflow?
> We still have the loop limit check, which checks that `limit <= max_int-stride`, and this means we would never enter the loop if we took the `Phi` branch that led to the overflow.
> 
> I could not just remove the assert, because in `LoopLimitNode::Ideal` we have this (strange?) check that does not optimize the `LoopLimitNode` if the inputs are constants:
> 
>   if (in(Init)->is_Con() && in(Limit)->is_Con())
>     return nullptr;  // Value
> 
> The assumption seems to be that we want `Value` to do the constant folding here - but of course we di...

You are right, but I believe the resulting expansion will constant fold regardless. So, why do we need to reject constant folding of the `LoopLimitNode` in the presence of overflow?

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

PR Comment: https://git.openjdk.org/jdk/pull/23024#issuecomment-2587100205