RFR: 8320909: C2 compilation fails with "Missed optimization opportunity in PhaseIterGVN"

[Marc Chevalier]

On Tue, 22 Apr 2025 08:06:48 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

> 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...

I've put only the (reduced) reproducers that I fix here (so 2 cases), including one that doesn't work anymore. I've not put the `Test`/`TestSimple` one, the one that is properly fixed by another PR, and is quite a different issue actually, simply hitting the same assert. Sure, I can add it. I just feared it would be confusing when browsing the history, to find a test actually fixed by another change than the one we will find in git blame.

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

PR Comment: https://git.openjdk.org/jdk/pull/24792#issuecomment-2820705595