On Mon, 14 Sep 2026 09:21:01 GMT, Roland Westrelin <[email protected]> wrote:
>> Kangcheng Xu has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - simplify logic by allowing deopt with size == 0
>> - remove left over code
>
> src/hotspot/share/opto/library_call.cpp line 1389:
>
>> 1387: // 1) length >= 0 (non-negative guard)
>> 1388: // 2) from u< length (range check, RCE-hoistable)
>> 1389: // 3) to - from - 1 u< length (range check, RCE-hoistable)
>
> This one checks `from < to`. I think you want to make it clearer in the
> comment.
> But if:
>
> `from = Integer.MAX_VALUE`
> `to = Integer.MIN_VALUE`
>
> then `to - from - 1 = 0`. So even though `from > to`, `to - from - 1 u<
> length` is true.
> This said, it looks harmless because `to - 1 u< length` fails (`to - 1 =
> Integer.MAX_VALUE` which can't be strictly less than any positive signed
> integer).
> Can you confirm, this is correct?
1. Updated comments with implied inequalities.
2. Yes, `to - 1 u< length` does catch it. Actually, check 2 also independently
catches it:
`from u< length` -> `MAX_VALUE u< length` -> *false* for any non-negative
signed length.
`checkFromToIndex(max, min, max)` has been added to hardcoded test cases.
> src/hotspot/share/opto/library_call.cpp line 1434:
>
>> 1432: Node* to_minus_one = _gvn.transform(SubNode::make(to,
>> _gvn.integercon(1, bt), bt));
>> 1433: Node* casted_to_minus_one = insert_unsigned_range_check(*this,
>> to_minus_one, casted_length, bt);
>> 1434: if (casted_to_minus_one == nullptr) {
>
> Couldn't you cast `to` to `[1..length-1]` here?
I believe you mean `[1, length]`? (i.e., `to <= length`) I added a cast node
with type `[max(1, old_lo), min(length_hi, old_hi)]` which only narrows bounds
instead widening it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r4038477308
PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r4038477448
