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

[Emanuel Peter]

On Wed, 17 Jan 2024 19:39:02 GMT, Joshua Cao <duke at openjdk.org> wrote:

>> src/hotspot/share/opto/loopTransform.cpp line 345:
>> 
>>> 343:   bool neg_inv1 = (n1->is_Sub() && !n1->is_Cmp() && inv1_idx == 2) ||
>>> 344:                   (n1->is_Cmp() && inv2_idx == 1 && n2->is_Sub());
>>> 345:   if (n1->is_Sub() && !n1->is_Cmp() && inv1_idx == 1) {
>> 
>> Would you mind adding some comments for this logic?
>
> I added a little comment block. Not sure how useful it is. I agree the code is hard to follow. Before all the changes for `Cmp`, I was able to make sense of it by following the comments at the top of the function.

Hmm, how about using these variables?

bool n1_is_add = n1->is_Add();
bool n1_is_sub = n1->is_Sub() && !n1->is_Cmp();
bool n1_is_cmp = n1->is_Cmp();

Then you can just comment this:
`// Determine whether x, inv1, or inv2 should be negative in the transformed expression`

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

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