RFR: 8303970: C2 can not merge homogeneous adjacent two If [v3]

[Cesar Soares Lucas]

On Tue, 14 Mar 2023 12:43:19 GMT, Yi Yang <yyang at openjdk.org> wrote:

>> Hi, can I have a review for this patch? It adds new Identity for BoolNode to lookup homogenous integer comparison, i.e. `Bool (CmpX a b)` is identity to `Bool (CmpX b a)`, in this way, we are able to merge the following two "identical" Ifs, which is not before.
>> 
>> 
>>     public static void test(int a, int b) { // ok, identical ifs, apply split_if
>>         if (a == b) {
>>             int_field = 0x42;
>>         } else {
>>             int_field = 42;
>>         }
>>         if (a == b) {
>>             int_field = 0x42;
>>         } else {
>>             int_field = 42;
>>         }
>>     }
>> 
>>     public static void test(int a, int b) { // do nothing
>>         if (a == b) {
>>             int_field = 0x42;
>>         } else {
>>             int_field = 42;
>>         }
>>         if (b == a) {
>>             int_field = 0x42;
>>         } else {
>>             int_field = 42;
>>         }
>>     }
>> 
>> 
>> Testing: tier1, appllication/ctw/modules
>
> Yi Yang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review from tobias

src/hotspot/share/opto/subnode.cpp line 1488:

> 1486: 
> 1487: static bool is_arithmetic_cmp(Node* cmp) {
> 1488:   if (!cmp->is_Cmp()) {

Perhaps just merge this if with the next one.

src/hotspot/share/opto/subnode.cpp line 1514:

> 1512:   Node* reverse_cmp = NULL;
> 1513:   if ((_test._test == BoolTest::eq || _test._test == BoolTest::ne) &&
> 1514:       (reverse_cmp = cmp->as_Cmp()->get_reverse_cmp()) != nullptr) {

Please factor out the assignment to reverse_cmp.

src/hotspot/share/opto/subnode.cpp line 1539:

> 1537:   Node *cmp1 = cmp->in(1);
> 1538:   Node *cmp2 = cmp->in(2);
> 1539:   if (!cmp1) {

Please change to "if (cmp1 == nullptr)".

test/hotspot/jtreg/compiler/c2/irTests/TestBackToBackIfs.java line 91:

> 89:     }
> 90: 
> 91:     @Run(test = {"test", "test1", "test2"})

Can you please add some tests with more than two "if-elses"? I think having a test with a few consecutive "if-else" with alternating operands (e.g., "a != b", "b != a", "a != b", ...) may be interesting.

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

PR: https://git.openjdk.org/jdk/pull/12978