On Thu, 20 Aug 2026 14:10:10 GMT, Kangcheng Xu <[email protected]> wrote:
>> Hi, >> >> This PR adds intrinsics to `Preconditions.checkFromToIndex()` and >> `Preconditions.checkFromIndexSize()` to produced optimized IR that uses >> `RangeCheck` node instead of implicit comparisons. >> `Preconditions.checkIndex()` is also refactored (without additional >> optimizations) to use the same helper function. >> >> Some common patterns where calling `checkFromToIndex` or >> `checkFromIndexSize` in a loop can have range checks in main loop eliminated >> completely and, therefore, enables empty loop removal. >> >> IR and correctness tests are included and passing. Additional tests on >> `Preconditions.checkIndex()` were also added. >> >> Thanks! >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Kangcheng Xu has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 43 commits: > > - Merge branch 'master' into check-index-sub-range > - Merge branch 'master' into check-index-sub-range > - fix TestRangeCheck > - enable intrinsic control for release builds > - fix TestOpaqueConstantBoolNodes > - adjust benchmark iterations > - Merge remote-tracking branch 'upstream/master' into > check-index-sub-range-bench > - update benchmarks > - update tests with deopt on length=max > - improve checkFromToIndex RCE hoisting > - ... and 33 more: https://git.openjdk.org/jdk/compare/7bc01544...30d2f0c7 Changes requested by roland (Reviewer). src/hotspot/share/opto/addnode.cpp line 273: > 271: } > 272: > 273: AddNode* AddNode::make_or(Node* in1, Node* in2, BasicType bt) { This doesn't appear to be used src/hotspot/share/opto/library_call.cpp line 78: > 76: #include "utilities/powerOfTwo.hpp" > 77: > 78: #include <initializer_list> Is this needed? src/hotspot/share/opto/library_call.cpp line 1332: > 1330: // Additionally, due to current RCE limitation on only recognizing > strict < (not <=), we workaround > 1331: // this issue by transforming `a u<= b` to `a u< b+1`. Therefore, the > updated checks are > 1332: // 1) size >= 0 (non-negative guard) Assuming the case where `size = 0` is not one we want to optimize for, what if we did: size > 0 length >= 0 from u< length from + size - 1 u< length ? It feels easier to reason about that one. src/hotspot/share/opto/library_call.cpp line 1381: > 1379: } > 1380: > 1381: replace_in_map(from, casted_from); Shouldn't there be a `replace_in_map` for `length` too? src/hotspot/share/opto/library_call.cpp line 1406: > 1404: // As explained in inline_preconditions_checkFromIndexSize(), the > limitation on RCE forces us to > 1405: // transform above checks to: > 1406: // 1) length + 1 > 0 (non-negative guard) Why not do: length >= 0 from < length to - from - 1 < length to - 1 < length ? src/hotspot/share/opto/library_call.cpp line 1426: > 1424: > 1425: // 1) length + 1 > 0 — guard ensuring length >= 0 and producing [1, > MAX] type for RCE. > 1426: // FIXME: RCE only recognizes patterns with strict <. We implement <= > by incrementing RHS. This Isn't that one supposed to be loop invariant and not need RCE? src/hotspot/share/opto/library_call.cpp line 1456: > 1454: } > 1455: > 1456: replace_in_map(from, casted_from); Same here: `replace_in_map` for `length`? ------------- PR Review: https://git.openjdk.org/jdk/pull/31138#pullrequestreview-4990878864 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3828293897 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3828295883 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3828538712 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3828327698 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3828601360 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3828566180 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3828331242
