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

Emanuel Peter epeter at openjdk.org
Fri Jan 10 07:15:06 UTC 2025


`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-8335676).

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 did not constant-fold because we had detected the overflow in `Value`. Not optimizing further here has a unfortunate consequence: on platforms that do not have `LoopLimit` implemented in the backend directly, we would have "lowered" the `LoopLimit` further down into `ConvI2L, SubL, AddL, DivL, ConvL2I ...` nodes. With this check, we do never lower it, and end up with a "bad AD", i.e. compilation bailout in product.

I think this check can reasonably be removed, because `Value` should be called before `Ideal` anyway, and so if we can constand fold because of constant inputs, we would have already done so.

Note, that the lowering is delayed until `post_loop_opts_phase`, but we never did `record_for_post_loop_opts_igvn`, and so it was not guaranteed that we actually ever processed the `LoopLimitNode` again, which would mean we got "bad AD" again, i.e. compilation bailout in product.

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

Commit messages:
 - typo in comment
 - copyright
 - Merge branch 'master' into JDK-8335747-LoopLimitNode-overflow
 - JDK-8335747

Changes: https://git.openjdk.org/jdk/pull/23024/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23024&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8335747
  Stats: 85 lines in 2 files changed: 76 ins; 3 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/23024.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/23024/head:pull/23024

PR: https://git.openjdk.org/jdk/pull/23024


More information about the hotspot-compiler-dev mailing list