8191887: assert(b->is_Bool()) in PhaseIdealLoop::clone_iff() due to Opaque4 node
Summary: add special handling for graph shape If->Opaque4->Bool->CmpP
Reviewed-by: kvn
--- a/src/hotspot/share/opto/loopnode.hpp Mon Nov 27 17:51:21 2017 -0500
+++ b/src/hotspot/share/opto/loopnode.hpp Mon Nov 27 16:05:11 2017 -0800
@@ -1098,7 +1098,7 @@
// "Nearly" because all Nodes have been cloned from the original in the loop,
// but the fall-in edges to the Cmp are different. Clone bool/Cmp pairs
// through the Phi recursively, and return a Bool.
- BoolNode *clone_iff( PhiNode *phi, IdealLoopTree *loop );
+ Node *clone_iff( PhiNode *phi, IdealLoopTree *loop );
CmpNode *clone_bool( PhiNode *phi, IdealLoopTree *loop );
--- a/src/hotspot/share/opto/loopopts.cpp Mon Nov 27 17:51:21 2017 -0500
+++ b/src/hotspot/share/opto/loopopts.cpp Mon Nov 27 16:05:11 2017 -0800
@@ -1407,47 +1407,56 @@
// "Nearly" because all Nodes have been cloned from the original in the loop,
// but the fall-in edges to the Cmp are different. Clone bool/Cmp pairs
// through the Phi recursively, and return a Bool.
-BoolNode *PhaseIdealLoop::clone_iff( PhiNode *phi, IdealLoopTree *loop ) {
+Node* PhaseIdealLoop::clone_iff(PhiNode *phi, IdealLoopTree *loop) {
// Convert this Phi into a Phi merging Bools
uint i;
- for( i = 1; i < phi->req(); i++ ) {
+ for (i = 1; i < phi->req(); i++) {
Node *b = phi->in(i);
- if( b->is_Phi() ) {
- _igvn.replace_input_of(phi, i, clone_iff( b->as_Phi(), loop ));
+ if (b->is_Phi()) {
+ _igvn.replace_input_of(phi, i, clone_iff(b->as_Phi(), loop));
} else {
- assert( b->is_Bool(), "" );
+ assert(b->is_Bool() || b->Opcode() == Op_Opaque4, "");
}
}
- Node *sample_bool = phi->in(1);
- Node *sample_cmp = sample_bool->in(1);
+ Node* n = phi->in(1);
+ Node* sample_opaque = NULL;
+ Node *sample_bool = NULL;
+ if (n->Opcode() == Op_Opaque4) {
+ sample_opaque = n;
+ sample_bool = n->in(1);
+ assert(sample_bool->is_Bool(), "wrong type");
+ } else {
+ sample_bool = n;
+ }
+ Node *sample_cmp = sample_bool->in(1);
// Make Phis to merge the Cmp's inputs.
- PhiNode *phi1 = new PhiNode( phi->in(0), Type::TOP );
- PhiNode *phi2 = new PhiNode( phi->in(0), Type::TOP );
- for( i = 1; i < phi->req(); i++ ) {
- Node *n1 = phi->in(i)->in(1)->in(1);
- Node *n2 = phi->in(i)->in(1)->in(2);
- phi1->set_req( i, n1 );
- phi2->set_req( i, n2 );
- phi1->set_type( phi1->type()->meet_speculative(n1->bottom_type()));
- phi2->set_type( phi2->type()->meet_speculative(n2->bottom_type()));
+ PhiNode *phi1 = new PhiNode(phi->in(0), Type::TOP);
+ PhiNode *phi2 = new PhiNode(phi->in(0), Type::TOP);
+ for (i = 1; i < phi->req(); i++) {
+ Node *n1 = sample_opaque == NULL ? phi->in(i)->in(1)->in(1) : phi->in(i)->in(1)->in(1)->in(1);
+ Node *n2 = sample_opaque == NULL ? phi->in(i)->in(1)->in(2) : phi->in(i)->in(1)->in(1)->in(2);
+ phi1->set_req(i, n1);
+ phi2->set_req(i, n2);
+ phi1->set_type(phi1->type()->meet_speculative(n1->bottom_type()));
+ phi2->set_type(phi2->type()->meet_speculative(n2->bottom_type()));
}
// See if these Phis have been made before.
// Register with optimizer
Node *hit1 = _igvn.hash_find_insert(phi1);
- if( hit1 ) { // Hit, toss just made Phi
+ if (hit1) { // Hit, toss just made Phi
_igvn.remove_dead_node(phi1); // Remove new phi
- assert( hit1->is_Phi(), "" );
+ assert(hit1->is_Phi(), "" );
phi1 = (PhiNode*)hit1; // Use existing phi
} else { // Miss
_igvn.register_new_node_with_optimizer(phi1);
}
Node *hit2 = _igvn.hash_find_insert(phi2);
- if( hit2 ) { // Hit, toss just made Phi
+ if (hit2) { // Hit, toss just made Phi
_igvn.remove_dead_node(phi2); // Remove new phi
- assert( hit2->is_Phi(), "" );
+ assert(hit2->is_Phi(), "" );
phi2 = (PhiNode*)hit2; // Use existing phi
} else { // Miss
_igvn.register_new_node_with_optimizer(phi2);
@@ -1457,8 +1466,8 @@
set_ctrl(phi2, phi->in(0));
// Make a new Cmp
Node *cmp = sample_cmp->clone();
- cmp->set_req( 1, phi1 );
- cmp->set_req( 2, phi2 );
+ cmp->set_req(1, phi1);
+ cmp->set_req(2, phi2);
_igvn.register_new_node_with_optimizer(cmp);
set_ctrl(cmp, phi->in(0));
@@ -1468,8 +1477,16 @@
_igvn.register_new_node_with_optimizer(b);
set_ctrl(b, phi->in(0));
- assert( b->is_Bool(), "" );
- return (BoolNode*)b;
+ if (sample_opaque != NULL) {
+ Node* opaque = sample_opaque->clone();
+ opaque->set_req(1, b);
+ _igvn.register_new_node_with_optimizer(opaque);
+ set_ctrl(opaque, phi->in(0));
+ return opaque;
+ }
+
+ assert(b->is_Bool(), "");
+ return b;
}
//------------------------------clone_bool-------------------------------------
@@ -1749,21 +1766,24 @@
// in the loop to break the loop, then test is again outside of the
// loop to determine which way the loop exited.
// Loop predicate If node connects to Bool node through Opaque1 node.
- if (use->is_If() || use->is_CMove() || C->is_predicate_opaq(use)) {
+ if (use->is_If() || use->is_CMove() || C->is_predicate_opaq(use) || use->Opcode() == Op_Opaque4) {
// Since this code is highly unlikely, we lazily build the worklist
// of such Nodes to go split.
- if( !split_if_set )
+ if (!split_if_set) {
split_if_set = new Node_List(area);
+ }
split_if_set->push(use);
}
- if( use->is_Bool() ) {
- if( !split_bool_set )
+ if (use->is_Bool()) {
+ if (!split_bool_set) {
split_bool_set = new Node_List(area);
+ }
split_bool_set->push(use);
}
- if( use->Opcode() == Op_CreateEx ) {
- if( !split_cex_set )
+ if (use->Opcode() == Op_CreateEx) {
+ if (!split_cex_set) {
split_cex_set = new Node_List(area);
+ }
split_cex_set->push(use);
}
@@ -1852,31 +1872,31 @@
// takes control from one or more OLD Regions (which in turn get from NEW
// Regions). In any case, there will be a set of Phis for each merge point
// from the IF up to where the original BOOL def exists the loop.
- if( split_if_set ) {
- while( split_if_set->size() ) {
+ if (split_if_set) {
+ while (split_if_set->size()) {
Node *iff = split_if_set->pop();
- if( iff->in(1)->is_Phi() ) {
- BoolNode *b = clone_iff( iff->in(1)->as_Phi(), loop );
+ if (iff->in(1)->is_Phi()) {
+ Node *b = clone_iff(iff->in(1)->as_Phi(), loop);
_igvn.replace_input_of(iff, 1, b);
}
}
}
- if( split_bool_set ) {
- while( split_bool_set->size() ) {
+ if (split_bool_set) {
+ while (split_bool_set->size()) {
Node *b = split_bool_set->pop();
Node *phi = b->in(1);
- assert( phi->is_Phi(), "" );
- CmpNode *cmp = clone_bool( (PhiNode*)phi, loop );
+ assert(phi->is_Phi(), "");
+ CmpNode *cmp = clone_bool((PhiNode*)phi, loop);
_igvn.replace_input_of(b, 1, cmp);
}
}
- if( split_cex_set ) {
- while( split_cex_set->size() ) {
+ if (split_cex_set) {
+ while (split_cex_set->size()) {
Node *b = split_cex_set->pop();
- assert( b->in(0)->is_Region(), "" );
- assert( b->in(1)->is_Phi(), "" );
- assert( b->in(0)->in(0) == b->in(1)->in(0), "" );
- split_up( b, b->in(0), NULL );
+ assert(b->in(0)->is_Region(), "");
+ assert(b->in(1)->is_Phi(), "");
+ assert(b->in(0)->in(0) == b->in(1)->in(0), "");
+ split_up(b, b->in(0), NULL);
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/unsafe/TestLoopUnswitching.java Mon Nov 27 16:05:11 2017 -0800
@@ -0,0 +1,102 @@
+/*
+ * Copyright (c) 2017, Red Hat, Inc. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8191887
+ * @summary loop cloning misses support for Opaque4 node
+ * @modules java.base/jdk.internal.misc:+open
+ *
+ * @run main/othervm -XX:-BackgroundCompilation TestLoopUnswitching
+ *
+ */
+
+import jdk.internal.misc.Unsafe;
+import java.lang.reflect.Field;
+import java.util.Arrays;
+
+public class TestLoopUnswitching {
+
+ static final jdk.internal.misc.Unsafe UNSAFE = Unsafe.getUnsafe();
+ static final long F_OFFSET;
+
+ static class A {
+ int f;
+ A(int f) {
+ this.f = f;
+ }
+ }
+
+ static {
+ try {
+ Field fField = A.class.getDeclaredField("f");
+ F_OFFSET = UNSAFE.objectFieldOffset(fField);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ static int test1(A[] arr, boolean flag1, boolean flag2) {
+ int res = 0;
+ for (int i = 0; i < 10; i++) {
+ A a = arr[i];
+ if (flag1) { // triggers unswitching
+ res += UNSAFE.getInt(a, F_OFFSET);
+ }
+ if (flag2) {
+ // Opaque4 node here is in the loop but If is out of the loop
+ res += UNSAFE.getInt(a, F_OFFSET);
+ break;
+ }
+ res += UNSAFE.getInt(a, F_OFFSET);
+ }
+ return res;
+ }
+
+ static int test2(A[] arr, boolean flag1, boolean flag2) {
+ int res = 0;
+ for (int i = 0; i < 10; i++) {
+ A a = arr[i];
+ if (flag1) { // triggers unswitching
+ res += UNSAFE.getInt(a, F_OFFSET);
+ }
+ if (flag2) {
+ // Opaque4 node here is out of the loop but Bool is in the the loop
+ res += UNSAFE.getInt(a, F_OFFSET);
+ break;
+ }
+ res += a.f;
+ }
+ return res;
+ }
+
+ static public void main(String[] args) {
+ A[] arr = new A[1000];
+ Arrays.fill(arr, new A(0x42));
+ for (int i = 0; i < 20000; i++) {
+ test1(arr, (i%2) == 0, (i%2) == 0);
+ test2(arr, (i%2) == 0, (i%2) == 0);
+ }
+ }
+
+}