RFR: 8305634: Renaming predicates, simple cleanups, and adding summary about current predicates [v3]

[Tobias Hartmann]

On Wed, 10 May 2023 13:33:10 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> This is the first patch in a series of patches to fix the remaining issues with Assertion/Skeleton Predicates ([JDK-8288981](https://bugs.openjdk.org/browse/JDK-8288981)). To achieve this task, we've decided to cleanup the naming scheme of predicates, cleanup the code operating on predicates (matching predicates, skipping over predicates etc.), and finally redesign the Assertion/Skeleton Predicates. While the basic idea stays the same, we are using new nodes and unified, cleaned up classes and methods to do the job. This redesign simplifies and only even makes it possible to fix the remaining known issues in a clean way.
>> 
>> To make reviewing the entire change easier, I've decided to split the work into several PRs.
>> 
>> This first PR includes the following _semantic-preserving_ changes:
>> - Establishing a new naming scheme for all the predicates found in C2 which makes it easier to talk about the various kinds of predicates:
>>   - Updating the code (variables, method names etc.) accordingly. 
>>   - Renaming "Skeleton Predicates" to "Assertion Predicates".
>>   - Including a summary of all predicates found in C2 in `loopPredicate.cpp`.
>>   - Capitalizing predicate names to better distinguish them in comments (e.g. "Parse Predicate" instead of "parse predicate").
>> - Change `class Predicates` -> `class ParsePredicates`. 
>> - Improving type information (e.g. using `IfProjNode` instead of `ProjNode`, using `ParsePredicateSuccessProj/ParsePredicateUncommonProj` typedefs for Parse Predicates etc.).
>> - Removing unused variables.
>> - Removing unnecessary checks.
>> - Code style fixes in touched code.
>> 
>> Instead of giving more background information about Assertion Predicates and why we need them here, I've decided to write a _blog post_ dedicated to Assertion Predicates and Loop Predication. This provides an overview and introduction which, hopefully, makes reviewing the PRs related to Assertion Predicates easier - at least on a higher level. 
>> 
>> The blog post can be found on my Github page at:
>> https://chhagedorn.github.io/jdk/2023/05/05/assertion-predicates.html
>> 
>> Thanks to @rwestrel, @eme64, and @TobiHartmann for your help with discussions, brainstormings, and pre-reviewing some of the changes here and in upcoming PRs.
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Emanuel's review

Great work in cleaning up / renaming this complex code, Christian. The changes look good to me.

Also, nice blog post. It was very helpful to refresh my memory.

src/hotspot/share/opto/loopPredicate.cpp line 46:

> 44:  * uncommon trap on the entry path to the loop. The old check inside the loop can be eliminated. If the condition of the
> 45:  * Hoisted Predicate fails at runtime, we'll execute the uncommon trap to avoid entering the loop which misses the check.
> 46:  * Loop Predication can currently remove array range check and loop invariant checks (such as null checks).

Suggestion:

 * Loop Predication can currently remove array range checks and loop invariant checks (such as null checks).

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

Marked as reviewed by thartmann (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13864#pullrequestreview-1424564260
PR Review Comment: https://git.openjdk.org/jdk/pull/13864#discussion_r1192410184