On Fri, 21 Aug 2026 07:35:38 GMT, Roland Westrelin <[email protected]> wrote:
>> 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 > > 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 Sorry. It's some left-over code. > src/hotspot/share/opto/library_call.cpp line 78: > >> 76: #include "utilities/powerOfTwo.hpp" >> 77: >> 78: #include <initializer_list> > > Is this needed? Left-over code will be removed. > 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? Ideally yes, but there isn't a type-improved `length` to replace with. There's only `casted_length_plus_one`. Although we know `length + 1 > 0 `, establishing `length > -1` can be tricky with over/underflow. I'd prefer avoid this unless this kind of type improvement is really beneficial. > 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? `1) length + 1 > 0` is loop invariant, but `2) from u<= length` and `3) (to - from) u<= length` are not. To achieve the `2)` and `3)` with strictly `<`, `length + 1` node is needed regardless. Please correct me if I misunderstood your comment. Thank you! > 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`? Same as above. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3831497649 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3831498058 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3831498612 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3831500068 PR Review Comment: https://git.openjdk.org/jdk/pull/31138#discussion_r3831498909
