RFR: 8347449: C2: UseLoopPredicate off should also turn UseProfiledLoopPredicate off [v5]

[Emanuel Peter]

On Thu, 27 Mar 2025 09:24:00 GMT, Manuel Hässig <duke at openjdk.org> wrote:

>> # Issue Summary
>> 
>> When running with `-XX:-UseLoopPredicate` C2 still inserts profiled loop parse predicates, despite those being a form of loop parse predicate. Further, the loop predicate code is not always consistent when to insert/expect profiled parse predicates.
>> 
>> # Change Summary
>> 
>> Following the rationale, that profiled predicates are a subset of loop predicates, this PR disables profiled predicates whenever loop predicates are disabled. They are disabled on the level of arguments. Further, before any checks for whether profiled predicates are enabled, this PR inserts a check that loop predicates are enabled such that the code is consistent in its intention.
>> 
>> Concretel, this PR
>>  - adds parse predicate nodes to the IR testing framework,
>>  - turns off `UseProfiledLoopPredicate` if `UseLoopPredicate` is turned off,
>>  - predicates all checks for `UseProfiledLoopPredicate`on `UseLoopPredicate` first for consistency,
>>  - adds a regression test.
>> 
>> 
>> # Testing
>>  
>> The changes passed the following testing:
>>  - [Github Actions](https://github.com/mhaessig/jdk/actions/runs/14078750038)
>>  - tier1 through tier3 and Oracle internal testing
>
> Manuel Hässig has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8347449-loop-predicate
>  - Improve help text for UseProfiledLoopPredicate argument
>  - loopnode: cleaner control flow
>  - Clean up IR test
>  - Apply suggestions from @chhagedorn
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>  - ir-framework: rename new nodes to convention
>  - ir-framework: fix phase for parse predicate nodes
>  - Make conditions on UseProfiledLoopPredicate first test UseLoopPredicate
>  - Turn off UseProfiledLoopPredicate when UseLoopPredicate is turned off
>  - Add regression IR test
>  - ... and 1 more: https://git.openjdk.org/jdk/compare/412d134a...72ebfc8e

Thanks for working on this!

You could also clean up the `IdealKit::loop`, which checks `UseLoopPredicate` only to call `add_parse_predicates`, which adds all predicates... and so it constrains too many things now.

src/hotspot/share/opto/c2_globals.hpp line 790:

> 788:           "Move checks with an uncommon trap out of loops based on "        \
> 789:           "profiling data. "                                                \
> 790:           "Requires UseLoopPredicate to be turned on (default).")           \

Can you also update the comment for `UseLoopPredicate`? It seems outdated / wrong.

Now is:
`Generate a predicate to select fast/slow loop versions`

@chhagedorn do you have a good suggestion for what to put now?

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

PR Review: https://git.openjdk.org/jdk/pull/24248#pullrequestreview-2721665164
PR Review Comment: https://git.openjdk.org/jdk/pull/24248#discussion_r2016743798