8231620: assert(bol->is_Bool()) crash during split if due to FastLockNode
Reviewed-by: vlivanov, thartmann
--- a/src/hotspot/share/opto/loopopts.cpp Mon Oct 07 12:06:47 2019 +0200
+++ b/src/hotspot/share/opto/loopopts.cpp Tue Oct 01 10:28:12 2019 +0200
@@ -1075,26 +1075,21 @@
// uses.
// A better fix for this problem can be found in the BugTraq entry, but
// expediency for Mantis demands this hack.
- // 6855164: If the merge point has a FastLockNode with a PhiNode input, we stop
- // split_if_with_blocks from splitting a block because we could not move around
- // the FastLockNode.
+#ifdef _LP64
for (DUIterator_Fast imax, i = region->fast_outs(imax); i < imax; i++) {
Node* n = region->fast_out(i);
if (n->is_Phi()) {
for (DUIterator_Fast jmax, j = n->fast_outs(jmax); j < jmax; j++) {
Node* m = n->fast_out(j);
- if (m->is_FastLock())
- return false;
-#ifdef _LP64
if (m->Opcode() == Op_ConvI2L)
return false;
if (m->is_CastII() && m->isa_CastII()->has_range_check()) {
return false;
}
-#endif
}
}
}
+#endif
return true;
}
--- a/src/hotspot/share/opto/split_if.cpp Mon Oct 07 12:06:47 2019 +0200
+++ b/src/hotspot/share/opto/split_if.cpp Tue Oct 01 10:28:12 2019 +0200
@@ -110,83 +110,90 @@
n->dump();
}
#endif
- // Clone down any block-local BoolNode uses of this CmpNode
- for (DUIterator i = n->outs(); n->has_out(i); i++) {
- Node* bol = n->out(i);
- assert( bol->is_Bool(), "" );
- if (bol->outcnt() == 1) {
- Node* use = bol->unique_out();
- if (use->Opcode() == Op_Opaque4) {
- if (use->outcnt() == 1) {
- Node* iff = use->unique_out();
- assert(iff->is_If(), "unexpected node type");
- Node *use_c = iff->in(0);
+ if (!n->is_FastLock()) {
+ // Clone down any block-local BoolNode uses of this CmpNode
+ for (DUIterator i = n->outs(); n->has_out(i); i++) {
+ Node* bol = n->out(i);
+ assert( bol->is_Bool(), "" );
+ if (bol->outcnt() == 1) {
+ Node* use = bol->unique_out();
+ if (use->Opcode() == Op_Opaque4) {
+ if (use->outcnt() == 1) {
+ Node* iff = use->unique_out();
+ assert(iff->is_If(), "unexpected node type");
+ Node *use_c = iff->in(0);
+ if (use_c == blk1 || use_c == blk2) {
+ continue;
+ }
+ }
+ } else {
+ // We might see an Opaque1 from a loop limit check here
+ assert(use->is_If() || use->is_CMove() || use->Opcode() == Op_Opaque1, "unexpected node type");
+ Node *use_c = use->is_If() ? use->in(0) : get_ctrl(use);
if (use_c == blk1 || use_c == blk2) {
+ assert(use->is_CMove(), "unexpected node type");
continue;
}
}
- } else {
- // We might see an Opaque1 from a loop limit check here
- assert(use->is_If() || use->is_CMove() || use->Opcode() == Op_Opaque1, "unexpected node type");
- Node *use_c = use->is_If() ? use->in(0) : get_ctrl(use);
- if (use_c == blk1 || use_c == blk2) {
- assert(use->is_CMove(), "unexpected node type");
- continue;
+ }
+ if (get_ctrl(bol) == blk1 || get_ctrl(bol) == blk2) {
+ // Recursively sink any BoolNode
+#ifndef PRODUCT
+ if( PrintOpto && VerifyLoopOptimizations ) {
+ tty->print("Cloning down: ");
+ bol->dump();
}
- }
- }
- if (get_ctrl(bol) == blk1 || get_ctrl(bol) == blk2) {
- // Recursively sink any BoolNode
-#ifndef PRODUCT
- if( PrintOpto && VerifyLoopOptimizations ) {
- tty->print("Cloning down: ");
- bol->dump();
- }
#endif
- for (DUIterator j = bol->outs(); bol->has_out(j); j++) {
- Node* u = bol->out(j);
- // Uses are either IfNodes, CMoves or Opaque4
- if (u->Opcode() == Op_Opaque4) {
- assert(u->in(1) == bol, "bad input");
- for (DUIterator_Last kmin, k = u->last_outs(kmin); k >= kmin; --k) {
- Node* iff = u->last_out(k);
- assert(iff->is_If() || iff->is_CMove(), "unexpected node type");
- assert( iff->in(1) == u, "" );
+ for (DUIterator j = bol->outs(); bol->has_out(j); j++) {
+ Node* u = bol->out(j);
+ // Uses are either IfNodes, CMoves or Opaque4
+ if (u->Opcode() == Op_Opaque4) {
+ assert(u->in(1) == bol, "bad input");
+ for (DUIterator_Last kmin, k = u->last_outs(kmin); k >= kmin; --k) {
+ Node* iff = u->last_out(k);
+ assert(iff->is_If() || iff->is_CMove(), "unexpected node type");
+ assert( iff->in(1) == u, "" );
+ // Get control block of either the CMove or the If input
+ Node *iff_ctrl = iff->is_If() ? iff->in(0) : get_ctrl(iff);
+ Node *x1 = bol->clone();
+ Node *x2 = u->clone();
+ register_new_node(x1, iff_ctrl);
+ register_new_node(x2, iff_ctrl);
+ _igvn.replace_input_of(x2, 1, x1);
+ _igvn.replace_input_of(iff, 1, x2);
+ }
+ _igvn.remove_dead_node(u);
+ --j;
+ } else {
+ // We might see an Opaque1 from a loop limit check here
+ assert(u->is_If() || u->is_CMove() || u->Opcode() == Op_Opaque1, "unexpected node type");
+ assert(u->in(1) == bol, "");
// Get control block of either the CMove or the If input
- Node *iff_ctrl = iff->is_If() ? iff->in(0) : get_ctrl(iff);
- Node *x1 = bol->clone();
- Node *x2 = u->clone();
- register_new_node(x1, iff_ctrl);
- register_new_node(x2, iff_ctrl);
- _igvn.replace_input_of(x2, 1, x1);
- _igvn.replace_input_of(iff, 1, x2);
+ Node *u_ctrl = u->is_If() ? u->in(0) : get_ctrl(u);
+ assert((u_ctrl != blk1 && u_ctrl != blk2) || u->is_CMove(), "won't converge");
+ Node *x = bol->clone();
+ register_new_node(x, u_ctrl);
+ _igvn.replace_input_of(u, 1, x);
+ --j;
}
- _igvn.remove_dead_node(u);
- --j;
- } else {
- // We might see an Opaque1 from a loop limit check here
- assert(u->is_If() || u->is_CMove() || u->Opcode() == Op_Opaque1, "unexpected node type");
- assert(u->in(1) == bol, "");
- // Get control block of either the CMove or the If input
- Node *u_ctrl = u->is_If() ? u->in(0) : get_ctrl(u);
- assert((u_ctrl != blk1 && u_ctrl != blk2) || u->is_CMove(), "won't converge");
- Node *x = bol->clone();
- register_new_node(x, u_ctrl);
- _igvn.replace_input_of(u, 1, x);
- --j;
}
+ _igvn.remove_dead_node(bol);
+ --i;
}
- _igvn.remove_dead_node(bol);
- --i;
}
}
// Clone down this CmpNode
for (DUIterator_Last jmin, j = n->last_outs(jmin); j >= jmin; --j) {
- Node* bol = n->last_out(j);
- assert( bol->in(1) == n, "" );
+ Node* use = n->last_out(j);
+ uint pos = 1;
+ if (n->is_FastLock()) {
+ pos = TypeFunc::Parms + 2;
+ assert(use->is_Lock(), "FastLock only used by LockNode");
+ }
+ assert(use->in(pos) == n, "" );
Node *x = n->clone();
- register_new_node(x, get_ctrl(bol));
- _igvn.replace_input_of(bol, 1, x);
+ register_new_node(x, ctrl_or_self(use));
+ _igvn.replace_input_of(use, pos, x);
}
_igvn.remove_dead_node( n );
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/loopopts/SplitIfSharedFastLockBehindCastPP.java Tue Oct 01 10:28:12 2019 +0200
@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2019, 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 8231620
+ * @summary assert(bol->is_Bool()) crash during split if due to FastLockNode
+ *
+ * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:-UseOnStackReplacement SplitIfSharedFastLockBehindCastPP
+ */
+
+
+public class SplitIfSharedFastLockBehindCastPP {
+ private static boolean field;
+ private static A obj_field;
+
+ public static void main(String[] args) {
+ A lock = new A();
+ obj_field = lock;
+ for (int i = 0; i < 20_000; i++) {
+ test1(true, lock);
+ test1(false, lock);
+ test2(true);
+ test2(false);
+ }
+ }
+
+ private static void test1(boolean flag, Object obj) {
+ if (obj == null) {
+ }
+
+ boolean flag2;
+ if (flag) {
+ flag2 = true;
+ } else {
+ flag2 = false;
+ obj = obj_field;
+ }
+
+ // This loop will be unswitched. The condition becomes candidate for split if
+ for (int i = 0; i < 100; i++) {
+ if (flag2) {
+ field = true;
+ } else {
+ field = false;
+ }
+ synchronized (obj) {
+ field = true;
+ }
+ }
+ }
+
+ private static Object test2(boolean flag) {
+ int integer;
+ if (flag) {
+ field = true;
+ integer = 1;
+ } else {
+ field = false;
+ integer = 2;
+ }
+
+ Object obj = integer;
+
+ // This loop will be unswitched. The condition becomes candidate for split if
+ for (int i = 0; i < 100; i++) {
+ if (integer == 1) {
+ field = true;
+ } else {
+ field = false;
+ }
+ synchronized (obj) {
+ field = true;
+ }
+ }
+ return obj;
+ }
+
+ private static final class A {
+ }
+}