RFR: 8324655: Identify integer minimum and maximum patterns created with if statements [v2]

[Jasmine Karthikeyan]

On Wed, 7 Feb 2024 09:41:54 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/cfgnode.cpp line 1802:
>> 
>>> 1800: 
>>> 1801:   // Ensure comparison is an integral type
>>> 1802:   if (opcode == Op_CmpI || opcode == Op_CmpL) {
>> 
>> Nicer would be "bailout" ifs:
>> Suggestion:
>> 
>>   if (opcode != Op_CmpI && opcode != Op_CmpL) { return nullptr; }
>>   bool is_long = opcode == Op_CmpL;
>> 
>>   // Only accept canonicalized le and lt comparisons
>>   int test = bol->_test._test;
>>   if (test != BoolTest::le && test != BoolTest::lt) { return nullptr; }
>> 
>> 
>> It is nice to have all the "bailout" checks as early as possible, and then do the "meat" of the optimization afterward.
>
> This also avoids nesting: nesting is difficult to read and understand. A sequence of "bailouts" is easier to read.

I agree that this would make it easier to understand the logic, I've made this change.

>> src/hotspot/share/opto/cfgnode.cpp line 1824:
>> 
>>> 1822:   } else {
>>> 1823:     return nullptr;
>>> 1824:   }
>> 
>> Maybe also have variables `phi_true/false`, would make the code easier to read.
>
> Hmm. Maybe you can refactor this, or explain somehow why exactly the conditions lead to `min_path = 1/2`

I've added new variables for `phi_true` and `phi_false`, and also added a comment explaining how those values are selected.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17574#discussion_r1499537719
PR Review Comment: https://git.openjdk.org/jdk/pull/17574#discussion_r1499539577