RFR: 8305638: Renaming and small clean-ups around predicates [v5]
Christian Hagedorn
chagedorn at openjdk.org
Tue Apr 23 13:48:57 UTC 2024
> **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:
> - Removing unused parameters
> - Renaming variables/parameters/methods
>
> Th...
Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
Fix useful Template Assertion Predicate marking
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/16877/files
- new: https://git.openjdk.org/jdk/pull/16877/files/f9f74276..2b646e15
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=16877&range=04
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=16877&range=03-04
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/16877.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/16877/head:pull/16877
PR: https://git.openjdk.org/jdk/pull/16877
More information about the hotspot-compiler-dev
mailing list