RFR: 8320909: C2 compilation fails with "Missed optimization opportunity in PhaseIterGVN"
Marc Chevalier
mchevalier at openjdk.org
Tue Apr 22 08:14:05 UTC 2025
The JBS issues has 3 reproducers. Two of them don't reproduce anymore. Let's enumerate:
- Test, TestSimple:
Disappeared with:
[JDK-8319372: C2 compilation fails with "Bad immediate dominator info"](https://bugs.openjdk.org/browse/JDK-8319372) in #16844
which actually fixed it by removing the handling of the problematic pattern in `CastIINode::Value`.
Reverting this fix makes the issue reappear.
- Reduced2: I fix here
- Test3, Reduced3:
Disappeared with:
[JDK-8347459: C2: missing transformation for chain of shifts/multiplications by constants](https://bugs.openjdk.org/browse/JDK-8347459) in #23728
which just shadowed it. The bug is essentially the same as Reduced2 otherwise. I also fix these here (even with reverted JDK-8347459)
The issue comes from the fact that `And[IL]Node::Value` has a special handling when
an operand is a left-shift: in the expression
lhs & (X << s)
if `lhs` fits in less than `s` bits, the result is sure to be 0. This special handling
also tolerate a `ConvI2LNode` between the `AndLNode` and `LShiftINode`. In this case,
updating the Shift node during IGVN won't enqueue directly the And node, but only the
Conv node. If this conv node cannot be improved, the And node is not enqueued, and its
type is not as good as it could be.
Such a case is illustrated on the following figures from Reduced2. Node `239 Phi` is a phi with a
dead branch, so the node is about to be eleminated. On the second figure, we can see
`152 LShiftI` taking its place. The node `243 ConvI2L` is enqueued, but no change happens
during its idealization, so the node `244 AndL` is not enqueued, while it could receive an update.
<img src="https://github.com/user-attachments/assets/a68b27d0-b3cc-4850-b20e-0cff17047ed8" width="400">
<img src="https://github.com/user-attachments/assets/d98731e8-d85c-413d-a77e-059787b01884" width="400">
The fix is pretty direct: we recognize this pattern and we enqueue the And node during IGVN
to make sure it has a chance to be refined.
The case of Reduced3 is mostly the same, with a twist: the special handling of And nodes
also can see through casts. See the next figure with a `CastIINode` between the `AndINode` and
the `LShiftINode`. The fix has to take that into account.
<img src="https://github.com/user-attachments/assets/ae3d5a01-a0d6-4346-944c-4ba46af64ee2" width="400">
Overall, the situation can be of the form:
LShift -> Cast+ -> ConvI2L -> Cast+ -> And
This second case was shadowed by [JDK-8347459](https://bugs.openjdk.org/browse/JDK-8347459) because the structure
And(constant, Cast(LShift(LShift(X, 16), 16)))
that appears in the example is simplified into
And(constant, Cast(0))
which enqueue the Cast for IGVN, but in this form it can improve its type: it's surely 0,
so the And node is also processed to deduce the value 0 as well.
I tried and failed to create a reproducer of this case that isn't shadowed by [JDK-8347459](https://bugs.openjdk.org/browse/JDK-8347459); in particular changing the constant from 16 to 15, for instance, doesn't allow to simplify the double shift into 0, but doesn't reproduce the issue.
One could imagine an alternate solution that would not update the notification system
but rather the type system. On top of intervals, we could have some modulo information
and know that some value is 0 modulo 2^s, or alternatively, some bitwise information
that would know the lower `s` bits to be 0 (and the top bits to be unknown). This
would propagate through casts and convs and join through phis. This would allow the And
node to have best type earlier on. If the information is already there (like on the
first figure). Also, the And node would not need to look for deep structures, but only
compare the modulo/bitwise information of operands to know if the value can be refined,
which means the enqueuing code would not need to be changed, and more cases would
naturally work without special pattern-matching from And node. Nevertheless, this solution
feels way out-of-scope...
Thanks,
Marc
-------------
Commit messages:
- Add timeout in test
- Looking through casts
- Add -XX:+IgnoreUnrecognizedVMOptions
- Add And to worklist through ConvI2L
Changes: https://git.openjdk.org/jdk/pull/24792/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24792&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8320909
Stats: 162 lines in 3 files changed: 162 ins; 0 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/24792.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/24792/head:pull/24792
PR: https://git.openjdk.org/jdk/pull/24792
More information about the hotspot-compiler-dev
mailing list