[8u-dev] Request for approval: 8164954: split_if creates empty phi and region nodes

[Rob McKenna]

Approved. Please add an appropriate noreg label.

    -Rob

On 13/10/17 23:09, Kevin Walls wrote:
> Hi,
> I'd like to get backport approval for 8u-dev for:
> bug: https://bugs.openjdk.java.net/browse/JDK-8164954
> 
> 9 changeset: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/38f38c10a11d
> 
> 9 Review thread: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-March/025773.html
> 
> Review of this 8u backport: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-October/027253.html
> 
> 
> It doesn't quite hg import cleanly as the surrounding code is a little different, but the code change itself is unchanged.
> 
> 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.
> 
> Thanks!
> Kevin
> 
> 
> 
> -------- Forwarded Message --------
> Subject: 	Re: [8u] RFF(S): 8164954: split_if creates empty phi and region
> nodes
> Date: 	Fri, 13 Oct 2017 10:57:46 -0700
> From: 	Vladimir Kozlov <vladimir.kozlov at oracle.com>
> To: 	Kevin Walls <kevin.walls at oracle.com>,
> hotspot-compiler-dev at openjdk.java.net
> 
> 
> 
> 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();
> >
> >
>