[8u] RFF(S): 8164954: split_if creates empty phi and region nodes

[Kevin Walls]

Thanks Vladimir!



On 13/10/2017 18:57, Vladimir Kozlov wrote:
> 8u change looks good.
>
> Thanks,
> Vladimir
>
> On 10/13/17 2:25 AM, Kevin Walls wrote:
>> Hi,
>>
>> I'd like to get a review of a backport to 8u.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8164954
>>
>> 9 changeset: 
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/38f38c10a11d
>>
>> Review thread: 
>> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-March/025773.html
>>
>>
>> It doesn't hg import cleanly as the surrounding code is a little 
>> different: this change adds a condition in split_if() which may make 
>> that method return earlier, but 8u does not have the block after the 
>> change, beginning "if (nb_predicate_proj > 1) {", that comes in with 
>> 8078426.
>>
>> The 8u change has been through jprt testing and also tested with the 
>> testsuite of a Java-based product which was seen hitting the same 
>> assert as in this bug.  hg diff of the proposed 8u change is below, I 
>> think that's enough but can offer a webrev if anybody needs one.
>>
>> Thanks!
>> Kevin
>>
>>
>> bash-4.2$ hg diff src/share/vm/opto/ifnode.cpp
>> diff -r c89173159237 src/share/vm/opto/ifnode.cpp
>> --- a/src/share/vm/opto/ifnode.cpp      Thu Sep 07 10:15:21 2017 -0400
>> +++ b/src/share/vm/opto/ifnode.cpp      Fri Oct 13 02:03:00 2017 -0700
>> @@ -234,6 +234,13 @@
>>         predicate_proj = proj;
>>       }
>>     }
>> +
>> +  // If all the defs of the phi are the same constant, we already 
>> have the desired end state.
>> +  // Skip the split that would create empty phi and region nodes.
>> +  if((r->req() - req_c) == 1) {
>> +    return NULL;
>> +  }
>> +
>>     Node* predicate_c = NULL;
>>     Node* predicate_x = NULL;
>>     bool counted_loop = r->is_CountedLoop();
>>
>>