RFR: 8305634: Renaming predicates, simple cleanups, and adding summary about current predicates [v4]
Emanuel Peter
epeter at openjdk.org
Mon May 15 08:14:46 UTC 2023
On Mon, 15 May 2023 06:09:47 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:
>
> Update src/hotspot/share/opto/loopPredicate.cpp
>
> Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
src/hotspot/share/opto/loopPredicate.cpp line 150:
> 148: * The Initialized Assertion Predicates are always true because we will
> 149: * never enter the main loop because of the changed pre- and main-loop
> 150: * exit conditions.
This does still not quite sound right. We will never enter the main loop? Sounds like the main-loop is ueseless in all cases. Suggestion:
The Initialized Assertion Predicates are always true: they are true when we enter the main loop
(because we adjusted the pre-loop exit condition), they are true in the last iteration (because we
adjust the main-loop exit condition), and they are true in all iterations in the middle by implication.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13864#discussion_r1193483805
More information about the hotspot-compiler-dev
mailing list