RFR: 8305638: Renaming and small clean-ups around predicates [v7]

[Christian Hagedorn]

On Thu, 2 May 2024 10:40:08 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> **Update: April 22**
>> 
>> After splitting off and integrating the following PRs from this PR:
>> https://github.com/openjdk/jdk/pull/18080
>> https://github.com/openjdk/jdk/pull/18293
>> https://github.com/openjdk/jdk/pull/18628
>> https://github.com/openjdk/jdk/pull/18723
>> 
>> we are only left with a few renaming and clean-ups from this PR. Directly merging the master branch in was quite hard. I therefore reverted all commits to get back to a clean master and then applied all remaining code changes manually (required a force push).
>> 
>>  <br>
>>  <br>
>> 
>> _------------ Original PR description --------------_
>> 
>> This patch is intended for JDK 23.
>> 
>> While preparing the patch for the full fix for Assertion Predicates [JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981), I still noticed that some changes are not required for the actual fix and could be split off and reviewed separately in this PR.
>> 
>> The patch applies the following cleanup changes:
>> - The complete fix had to add slightly different cloning cases in `PhaseIdealLoop::create_bool_from_template_assertion_predicate()` which already has quite some logic to switch between different cases. Additionally, the algorithm in the method itself was already hard to understand and difficult to adapt. I therefore re-implemented it in a separate class `CloneTemplateAssertionPredicateBool` together with some helper classes like `DFSNodeStack`. To use it, I've added a `TemplateAssertionPredicateBool` class that offers three cloning possibilities:
>>   - `clone()`: Clone without modification
>>   -  `clone_and_replace_opaque_loop_nodes()`: Clone and replace the `OpaqueLoop*Nodes` with a new init and stride node.
>>   - `clone_and_replace_init()`: Special case of `clone_and_replace_opaque_loop_nodes()` which only replaces `OpaqueLoopInitNode` and clones `OpaqueLoopStrideNode`.
>>   
>>   This refactoring could be extracted from the complete fix.
>> - The Split If code to detect (`subgraph_has_opaque()`) and clone Template Assertion Predicate Bools was extracted to a separate class `CloneTemplateAssertionPredicateBoolDown` and uses the new `TemplateAssertionPredicateBool` class to do the actual cloning.
>> - In the process of coding the complete fix, I've refactored the Loop Unswitching code quite a bit. This change could also be extracted into a separate RFE. Changes include:
>>   - Renaming
>>   - Extracting code to separate classes/methods
>>   - Adding comments
>> - Some small refactoring including:
>>   - Removi...
>
> Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
> 
>  - Merge branch 'master' into JDK-8305638
>  - Merge branch 'refs/heads/master' into JDK-8305638
>    
>    # Conflicts:
>    #	src/hotspot/share/opto/loopPredicate.cpp
>  - Fix useful Template Assertion Predicate marking
>  - Fix useful Parse Predicate marking
>  - Remaining renaming and small clean-ups

Thanks Roland for re-reviewing it!

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

PR Comment: https://git.openjdk.org/jdk/pull/16877#issuecomment-2095381632