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

[Rahul Raghavan]

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/

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
> >