RFR: 8344171: Clone and initialize Assertion Predicates in order instead of in reverse-order [v3]
Vladimir Kozlov
kvn at openjdk.org
Tue Nov 26 17:36:42 UTC 2024
On Mon, 25 Nov 2024 20:29:32 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
>> This patch changes the order in which we clone and initialize Assertion Predicates from "reverse-order" to "in-order".
>>
>> #### Current State: Mostly "reverse-order" for Assertion Predicates
>> We are currently cloning and initializing Assertion Predicates in reverse-order out of convenience and simplicity for most of the loop splitting optimizations - except for Loop Unswitching (see next section). This means that we do the following:
>>
>> old target loop entry
>> |
>> x Cloned Template Assertion
>> | Predicate 2
>> Template Assertion |
>> Predicate 1 Initialized Assertion
>> | ==> Predicate 2
>> Template Assertion |
>> Predicate 2 Cloned Template Assertion
>> | Predicate 1
>> source loop |
>> Initialized Assertion
>> Predicate 1
>> |
>> target loop
>>
>> I don't think this is wrong but still kinda unexpected when trying to reason about a graph. But now with the recent refactorings, I think it's easy to change this to an in-order processing:
>>
>> old target loop entry
>> |
>> x Cloned Template Assertion
>> | Predicate 1
>> Template Assertion |
>> Predicate 1 Initialized Assertion
>> | ==> Predicate 1
>> Template Assertion |
>> Predicate 2 Cloned Template Assertion
>> | Predicate 2
>> source loop |
>> Initialized Assertion
>> Predicate 2
>> |
>> target loop
>>
>> This will also align all cloning/initializing of Assertion Predicates to the same order which was not the case before: Loop Unswitching already had an in...
>
> 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' of https://git.openjdk.org/jdk into JDK-8344171
> - Update comment
> - 8344171: Clone and initialize Assertion Predicates in order instead of in reverse-order
> - Apply suggestions from code review
>
> Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
> - 8344213: Cleanup OpaqueLoop*Node verification code for Assertion Predicates
Seems fine to me.
-------------
Marked as reviewed by kvn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22275#pullrequestreview-2462275300
More information about the hotspot-compiler-dev
mailing list