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

[Xin Liu]

On Mon, 22 Jan 2024 16:54:57 GMT, Joshua Cao <duke at openjdk.org> wrote:

>> src/hotspot/share/opto/loopTransform.cpp line 382:
>> 
>>> 380:     }
>>> 381:     phase->register_new_node(inv, phase->get_early_ctrl(inv));
>>> 382:     if (n1_is_cmp) {
>> 
>> CmpNode is subclass of SubNode. if n is CmpI/L, n->is_Sub() is also true. 
>> can we use the old logic for your new comparison expressions? 
>> 
>> yes, we still need to check if n1 is CmpNode or SubNode here.
>
> I am not following. We are explicitly checking for Cmp here. Why do we also need to check if it is a Sub?

I think CmpNode has the same re-association rule as SubNode. 
take your own example, 
>  inv1 == (x - inv2) => ( inv1 + inv2 ) == x

Cmp(inv1, (x-inv2)) => Eq(0, Sub(inv1, (x-inv2)) => Eq(0, Sub(inv1+inv2, x)) => Cmp(inv1+inv2, x)

Originally, n1->is_Sub() covers both CmpNode and SubNode. I don't you need to split them it into 2 cases. 

I think you only to check if n1 Cmp or Sub when you are going to return Cmp/SubNode

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17375#discussion_r1462611394