RFR: 8354282: C2: more crashes in compiled code because of dependency on removed range check CastIIs [v8]

[Emanuel Peter]

On Tue, 2 Dec 2025 09:20:41 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> This is a variant of 8332827. In 8332827, an array access becomes
>> dependent on a range check `CastII` for another array access. When,
>> after loop opts are over, that RC `CastII` was removed, the array
>> access could float and an out of bound access happened. With the fix
>> for 8332827, RC `CastII`s are no longer removed.
>> 
>> With this one what happens is that some transformations applied after
>> loop opts are over widen the type of the RC `CastII`. As a result, the
>> type of the RC `CastII` is no longer narrower than that of its input,
>> the `CastII` is removed and the dependency is lost.
>> 
>> There are 2 transformations that cause this to happen:
>> 
>> - after loop opts are over, the type of the `CastII` nodes are widen
>>   so nodes that have the same inputs but a slightly different type can
>>   common.
>>   
>> - When pushing a `CastII` through an `Add`, if of the type both inputs
>>   of the `Add`s are non constant, then we end up widening the type
>>   (the resulting `Add` has a type that's wider than that of the
>>   initial `CastII`).
>>   
>> There are already 3 types of `Cast` nodes depending on the
>> optimizations that are allowed. Either the `Cast` is floating
>> (`depends_only_test()` returns `true`) or pinned. Either the `Cast`
>> can be removed if it no longer narrows the type of its input or
>> not. We already have variants of the `CastII`:
>> 
>> - if the Cast can float and be removed when it doesn't narrow the type
>> of its input.
>> 
>> - if the Cast is pinned and be removed when it doesn't narrow the type
>> of its input.
>> 
>> - if the Cast is pinned and can't be removed when it doesn't narrow
>> the type of its input.
>> 
>> What we need here, I think, is the 4th combination:
>> 
>> - if the Cast can float and can't be removed when it doesn't narrow
>> the type of its input.
>> 
>> Anyway, things are becoming confusing with all these different
>> variants named in ways that don't always help figure out what
>> constraints one of them operate under. So I refactored this and that's
>> the biggest part of this change. The fix consists in marking `Cast`
>> nodes when their type is widen in a way that prevents them from being
>> optimized out.
>> 
>> Tobias ran performance testing with a slightly different version of
>> this change and there was no regression.
>
> Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 17 commits:
> 
>  - Merge branch 'master' into JDK-8354282
>  - whitespace
>  - review
>  - review
>  - Update src/hotspot/share/opto/castnode.cpp
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>  - Update src/hotspot/share/opto/castnode.cpp
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>  - Update src/hotspot/share/opto/castnode.cpp
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>  - Update test/hotspot/jtreg/compiler/c2/irTests/TestPushAddThruCast.java
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>  - review
>  - review
>  - ... and 7 more: https://git.openjdk.org/jdk/compare/ef5e744a...93b8b0c5

@rwestrel Nice work! We not just only fixed the bug but made the concepts much clearer. This makes me very happy 😊

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

Marked as reviewed by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24575#pullrequestreview-3531172652