[jdk17u-dev] RFR: 8307683: Loop Predication should not hoist range checks with trap on success projection by negating their condition

[Severin Gehwolf]

On Thu, 13 Jul 2023 09:29:48 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>> This fixes a regression in 17.0.7.  To work around the regression, JDK-8297951 was backed out in 17.0.8.
>> 
>> Big parts of this change is passing a Proj node instead of the If that is the predecessor of the Proj through method calls. As methods differ in their arguments in 17, all these had to be resolved.
>> The real fix is in loop_predication_impl_helper().
>> 
>> Resolves in detail:
>> 
>> src/hotspot/share/opto/loopPredicate.cpp
>> In head, there are two variants of is_range_check_if(), 17 has only one. Omitted the changes to the second one.
>> loop_predication_impl_helper()
>> Renamed the variable. In head, it is if_proj->if_success_proj, here it is proj->success_proj.
>> I introduce a new vairalbe IfProjNode if_success_proj. Calls to loop_predication_impl_helper
>> pass Projs and not IfProjs, so this seems cleaner.  Added assertion.
>> Passing proj instead of if to is_range_check_if().
>> Computation of the deleted "bool negate" differs.  Deleted anyways.
>> Removed all the uses of negate.
>> 
>> src/hotspot/share/opto/loopTransform.cpp
>> Trivial resolve.
>> 
>> src/hotspot/share/opto/loopnode.cpp
>> extract_long_range_checks() was introduced in "8259609: C2: optimize long range checks in long counted loops".
>> The change is only needed as the input to is_range_check_if() was
>> changed from the IfNode to the IfProjNode below. The change here has no effect on the fix. Skipped.
>> 
>> patching file src/hotspot/share/opto/loopnode.hpp
>> Resolved, simple differences.
>
> In case you make a build with this and the redo change, you will need to do a simple build fix renaming a variable.

@GoeLin Testing looks good on my end. Thanks!

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

PR Comment: https://git.openjdk.org/jdk17u-dev/pull/1553#issuecomment-1634409383