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

[Roland Westrelin]

On Thu, 6 Jul 2023 07:11:22 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.

Looks good to me.

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

Marked as reviewed by roland (Reviewer).

PR Review: https://git.openjdk.org/jdk17u-dev/pull/1553#pullrequestreview-1523700990