RFR: 8307683: Loop Predication wrongly hoists IfNodes without a range check pattern as range check [v2]

[Severin Gehwolf]

On Fri, 26 May 2023 23:34:26 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> [JDK-4809552](https://bugs.openjdk.org/browse/JDK-4809552) allowed Loop Predication to be applied to `IfNodes` that have a positive value instead of a `LoadRangeNode`:
>> https://github.com/openjdk/jdk/blob/48d21bd089a3f344ee5407926f8ed2af3734d2b0/src/hotspot/share/opto/loopPredicate.cpp#L854-L862
>> 
>> This, however, is only correct if we have an actual `RangeCheckNode` for an array. The reason for that is that if we hoist a real range check and create a Hoisted Predicate for it, we only need to check the lower and upper bound of all array accesses (i.e. the array access of the first and the last loop iteration). All array accesses in between are implicitly covered and do not need to be checked again. 
>> 
>> But if we face an `IfNode` without a `LoadRangeNode`, we could be comparing anything. We do not have any guarantee that if the first and last loop iteration check succeed that the other loop iteration checks will also succeed. An example of this is shown in the test case `test()`. We wrongly create a Hoisted Range Check Predicate where the lower and upper bound are always true, but for some values of the loop induction variable, the hoisted check would actually fail. We then crash because an added Assertion Predicate exactly performs this failing check (crash with halt). Without any loop splitting (i.e. no Assertion Predicates), we have a wrong execution due to never executing the branch where we increment `iFld2` because we removed it together with the check.
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - fix assertion
>  - new fix with bailout for "if iv <u limit then trap"

This fails the `[compiler/loopopts/TestSkeletonPredicateNegation` test with:


#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/home/runner/work/jdk/jdk/src/hotspot/share/opto/loopPredicate.cpp:848), pid=32813, tid=32828
#  assert(!iff->is_RangeCheck()) failed: can only be IfNode because RangeCheckNodes always have trap on false projection
#
# JRE version: OpenJDK Runtime Environment (21.0) (fastdebug build 21-internal-chhagedorn-229583b7613867127e42baca158773bcf9c08c73)
# Java VM: OpenJDK 64-Bit Server VM (fastdebug 21-internal-chhagedorn-229583b7613867127e42baca158773bcf9c08c73, mixed mode, tiered, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x1298438]  IdealLoopTree::is_range_check_if(IfProjNode*, PhaseIdealLoop*, BasicType, Node*, Node*&, Node*&, long&) const+0x278
#
# CreateCoredumpOnCrash turned off, no core file dumped
#
# An error report file with more information is saved as:
# /home/runner/work/jdk/jdk/build/run-test-prebuilt/test-support/jtreg_test_hotspot_jtreg_tier1_compiler/scratch/hs_err_pid32813.log
#
# Compiler replay data is saved as:
# /home/runner/work/jdk/jdk/build/run-test-prebuilt/test-support/jtreg_test_hotspot_jtreg_tier1_compiler/scratch/replay_pid32813.log
#
# If you would like to submit a bug report, please visit:
#   https://bugreport.java.com/bugreport/crash.jsp
#
result: Error. Agent communication error: java.io.EOFException; check console log for any additional details


which seems related?

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

PR Comment: https://git.openjdk.org/jdk/pull/14156#issuecomment-1568019076