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

[Kevin Walls]

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();
> 
>