RFR: 8323220: Reassociate loop invariants involved in Cmps and Add/Subs [v5]

[Emanuel Peter]

On Wed, 24 Jan 2024 23:24:48 GMT, Joshua Cao <duke at openjdk.org> wrote:

>> // inv1 == (x + inv2) => ( inv1 - inv2 ) == x
>> // inv1 == (x - inv2) => ( inv1 + inv2 ) == x
>> // inv1 == (inv2 - x) => (-inv1 + inv2 ) == x
>> 
>> 
>> For example,
>> 
>> 
>> fn(inv1, inv2)
>>   while(...)
>>       x = foobar()
>>       if inv1 == x + inv2
>>            blackhole()
>> 
>> 
>> We can transform this into
>> 
>> 
>> fn(inv1, inv2)
>>   t = inv1 - inv2
>>   while(...)
>>       x = foobar()
>>       if t == x
>>            blackhole()
>> 
>> 
>> Here is an example: https://github.com/openjdk/jdk/blob/b78896b9aafcb15f453eaed6e154a5461581407b/src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java#L910. LHS `1` and RHS `pos` are both loop invariant
>> 
>> Passes tier1 locally on Linux machine. Passes GHA on my fork.
>
> Joshua Cao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   reassociate_add_sub -> reassociate_add_sub_cmp

Tests pass, thanks for the updates.
I left a few optional comments.

One requiest I would still like to see:
You could improve the tests, by returning a value that indicates when the loop was exited, i.e. `return i`. In the future, I intend to verify the return values from test methods, and then we would have additional coverage for free ;)

src/hotspot/share/opto/loopnode.hpp line 745:

> 743:   // Reassociate invariant binary expressions.
> 744:   Node* reassociate(Node* n1, PhaseIdealLoop *phase);
> 745:   // Reassociate invariant add and subtract expressions.

I guess you could mention cmp here too.

test/hotspot/jtreg/compiler/loopopts/InvariantCodeMotionReassociateCmp.java line 2:

> 1: /*
> 2:  * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.

Does the Amazon copyright header not have a year  associated with it?

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

Marked as reviewed by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17375#pullrequestreview-1843206952
PR Review Comment: https://git.openjdk.org/jdk/pull/17375#discussion_r1466069682
PR Review Comment: https://git.openjdk.org/jdk/pull/17375#discussion_r1466071003