RFR: 8144484: assert(no_dead_loop) failed: dead loop detected

[Vladimir Kozlov]

On 2/2/17 8:59 AM, Rahul Raghavan wrote:
> Hi,
>
> Thank you Vladimir for the review comments.
> Understood your point and agree that for the reported scenario
> bailing out early and delaying the optimization should be the fix.
>
> Please review updated <webrev.02> - http://cr.openjdk.java.net/~rraghavan/8144484/webrev.02/

Good.

Thanks,
Vladimir

>
> Confirmed no issues with testing
> (8144484 test, jprt -testset hotspot; so far no issues with RBT Pre-integration testing in progress )
>
> Thanks,
> Rahul
>
>> -----Original Message-----
>> From: Vladimir Kozlov
>> Sent: Wednesday, February 01, 2017 2:32 PM
>> To: hotspot-compiler-dev at openjdk.java.net
>> Subject: Re: RFR: 8144484: assert(no_dead_loop) failed: dead loop detected
>>
>> On 2/1/17 12:19 AM, Vladimir Kozlov wrote:
>>> Sorry, I don't like this fix.
>>>
>>> Based on your evaluation one control path is dead (top) and it will be optimized out later. It was unfortunate that Phi
>>> node was processed first. Change graph (split through MergeMem) is dangerous in this state of graph.
>>>
>>> I would suggest to bailout early:
>>>
>>> 1886   if (progress == NULL && can_reshape && type() == Type::MEMORY) {
>>> 1887     // see if this phi should be sliced
>>> 1888     uint merge_width = 0;
>>> 1889     bool saw_self = false;
>>> 1890     for( uint i=1; i<req(); ++i ) {// For all paths in
>>> 1891       Node *ii = in(i);
>>>
>>> +          // TOP inputs should not be counted as safe inputs because if the
>>> +          // Phi references itself through all other inputs then splitting the
>>> +          // Phi through memory merges would create dead loop at later stage.
>>> +          if (ii == top) {
>>> +            return top; // Delay optimization until graph is cleaned.
>>
>> Sorry, wrong return value. Should be return NULL;
>>
>>> +          }
>>>
>>> 1892       if (ii->is_MergeMem()) {
>>> 1893         MergeMemNode* n = ii->as_MergeMem();
>>>
>>> Thanks,
>>> Vladimir
>>>