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

src/hotspot/share/opto/castnode.hpp line 108:

> 106:     // Floating: The Cast is only dependent on the single range check.
> 107:     // Narrowing: The Cast narrows the type to a positive index. If the input to the Cast is narrower, we can safely
> 108:     //            remove the cast because the array access will be safe.

The "Floating" part is a bit counter intuitive here, because the ctrl of the CastII is the RangeCheck, right?
So is it not therefore already pinned?

Maybe we can add some detail about what the "floating" explicitly means here. Is it that we can later move the CastII up in an optimization?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24575#discussion_r2581630546