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

[Emanuel Peter]

On Thu, 8 Feb 2024 21:39:35 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:
> 
>   Add correctness test for some random tests with random inputs

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

> 35:  */
> 36: public class InvariantCodeMotionReassociateCmp {
> 37:     private static final int size = 500;

You could even randomize size in a `@Setup` method, and pass it as another argument.

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

> 52:             // Reassociate to `inv2 - inv1 == i`
> 53:             if (inv1 + i == inv2) {
> 54:                 blackhole();

If you return instead of break, do you still need the `blackhole`?

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

> 86:         if (returnValue[0] != size && returnValue[0] + returnValue[1] != returnValue[2]) {
> 87:             throw new RuntimeException("Illegal reassociation: i=" + returnValue[0] + ", inv1=" + returnValue[1]
> 88:                     + ", inv2=" + returnValue[2]);

Sadly, you cannot reuse `@Check` methods for multiple `@Test` methods. But you can have a utility method that you invoke from multiple `@Check` methods. That would reduce the code-duplication.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17375#discussion_r1483901005
PR Review Comment: https://git.openjdk.org/jdk/pull/17375#discussion_r1483901310
PR Review Comment: https://git.openjdk.org/jdk/pull/17375#discussion_r1483900598