RFR: 8355574: Fatal error in abort_verify_int_in_range due to Invalid CastII [v4]

[Vladimir Ivanov]

On Sun, 18 May 2025 07:06:41 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>> Hi,
>> 
>> The issue here is that the `CastLLNode` is created before the actual check that ensures the range of the input. This patch fixes it by moving the creation to the correct place, which is under `inline_block`. I also noticed that the code there seems incorrect and confusing. `ArrayCopyNode::get_partial_inline_vector_lane_count` takes the length of the array, not the size in bytes. If you look into the method it will multiply `const_len` with `type2aelementbytes(bt)` to get the size in bytes of the array. In the runtime test, we compare `length << log2(type2bytes(bt))` with `ArrayOperationPartialInlineSize`. This seems confusing, why don't we just compare `length` with `ArrayOperationPartialInlineSize / type2bytes(bt)`, it also unifies the test with the actual cast.
>> 
>> Please take a look and leave your reviews, thanks a lot.
>
> Quan Anh Mai has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - fix comment
>  - fix comment

Overall, looks good.

src/hotspot/share/opto/macroArrayCopy.cpp line 209:

> 207:   int inline_limit = ArrayOperationPartialInlineSize / type2aelembytes(type);
> 208: 
> 209:   const TypeLong* length_type = _igvn.type(length)->isa_long();

Any particular benefit in eagerly pruning the block? It duplicates post-expansion GVN checks of the branch condition. (If it were normal parsing with prompt GVN analysis, you could detect the branch is dead right after `generate_guard` call.) 

Alternatively, the checks are equivalent to checking that join of `length_type` with `[0...inline_limit]` is not empty. But I prefer to let GVN handle it.

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

PR Review: https://git.openjdk.org/jdk/pull/25284#pullrequestreview-2866012522
PR Review Comment: https://git.openjdk.org/jdk/pull/25284#discussion_r2105660733