RFR: 8305636: Expand and clean up predicate classes and move them into separate files [v3]

Christian Hagedorn chagedorn at openjdk.org
Thu Aug 3 08:07:26 UTC 2023


> This is the third clean-up PR towards fixing issues with Assertion Predicates ([JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981)). This patch does not change anything in the way the old Assertion Predicates work.
> 
> After collecting and moving the predicate code in the last clean-up PR https://github.com/openjdk/jdk/pull/14017 to the classes `Predicates/ParsePredicates`, I'm now completely moving the code to separate `predicates.cpp/hpp` files. By doing so, I also updated the predicate description and updated some namings. Since this description is also moved to the new files, I've committed the description update separately to better reflect these changes.
> 
> Changes include:
> - Moved `Predicates/ParsePredicates` classes to new files `predicates.cpp/hpp`.
> - Turning the `Predicates` utility class into a real class to represent all predicates:
>   - Contains three `PredicateBlock` fields for each Predicate Block (see description of `Predicate Block`).
>     - The `PredicateBlock` class offers methods to query the presence of predicates and to access them (e.g. get the Parse Predicate projection).
>   - In the process, the `ParsePredicates` could be removed as the Parse Predicates are now covered by the `PredicateBlock` class.
> - New `AssertionPredicatesWithHalt` class to skip over Assertion Predicates (will be further cleaned up later with the complete fix in JDK-8288981).
> - Updated predicate description and moved to `predicates.hpp`.
>   - While testing a prototype fix of JDK-8288981, I've came to the conclusion that we should not move all Assertion Predicates to a separate block below the Parse and Hoisted Predicates, because it prevented further application of Loop Predication due to pins of data nodes to these Assertion Predicates while the Hoisted Predicates needed them above the Assertion Predicates (i.e. dominance problems leading to bad graph assertions). I've removed that part of the description that gave a heads-up about that change.
> - Small clean-ups such as variable renaming or code move.
> 
> Not included:
> - Refactoring predicate traversal to clone/copy/initialize predicates for loop unswitching, pre/main/post, loop peeling etc. (this is only done in the actual fix in JDK-8288981 which requires some updates anyways - so this refactoring is not done here (yet)).
> 
> Testing: Tier1-7, hs-precheckin-comp, hs-comp-stress
> 
> Thanks,
> Christian

Christian Hagedorn 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 seven additional commits since the last revision:

 - Merge branch 'master' into JDK-8305636
 - 8308682: Enhance AES performance
   
   Reviewed-by: adinn, sviswanathan, dlong, kvn
 - 8308682: Enhance AES performance
 - Update src/hotspot/share/opto/predicates.hpp
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - Renaming Hoisted Predicate -> Hoisted Check Predicate in description and comments as discussed offline with Tobias, fixing additional typos in description
 - 8305636: Expand and clean up predicate classes and move them into separate files
 - Update description

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/14814/files
  - new: https://git.openjdk.org/jdk/pull/14814/files/020cff3c..b8d7759d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=14814&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14814&range=01-02

  Stats: 90638 lines in 1161 files changed: 27823 ins; 59504 del; 3311 mod
  Patch: https://git.openjdk.org/jdk/pull/14814.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14814/head:pull/14814

PR: https://git.openjdk.org/jdk/pull/14814


More information about the hotspot-compiler-dev mailing list