Skip to content

Commit

Permalink
8262017: C2: assert(n != __null) failed: Bad immediate dominator info.
Browse files Browse the repository at this point in the history
8205407: [windows, vs<2017] C4800 after 8203197

Reviewed-by: mbalao, roland
Backport-of: 2db9005c07585b580b3ec0889b8b5e3ed0d0ca6a
  • Loading branch information
franferrax authored and martinuy committed Jun 28, 2024
1 parent 44eac48 commit 385e34d
Show file tree
Hide file tree
Showing 5 changed files with 308 additions and 13 deletions.
2 changes: 1 addition & 1 deletion hotspot/make/windows/makefiles/compile.make
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ CXX=cl.exe
# improving the quality of crash log stack traces involving jvm.dll.

# These are always used in all compiles
CXX_FLAGS=$(EXTRA_CFLAGS) /nologo /W3 /WX
CXX_FLAGS=$(EXTRA_CFLAGS) /nologo /W3 /WX /wd4800

# Let's add debug information when Full Debug Symbols is enabled
!if "$(ENABLE_FULL_DEBUG_SYMBOLS)" == "1"
Expand Down
89 changes: 89 additions & 0 deletions hotspot/src/share/vm/opto/addnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,95 @@ const Type *XorLNode::add_ring( const Type *t0, const Type *t1 ) const {
return TypeLong::make( r0->get_con() ^ r1->get_con() );
}


Node* MaxNode::build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn) {
bool is_int = gvn.type(a)->isa_int();
assert(is_int || gvn.type(a)->isa_long(), "int or long inputs");
assert(is_int == (gvn.type(b)->isa_int() != NULL), "inconsistent inputs");
if (!is_unsigned) {
if (is_max) {
if (is_int) {
Node* res = gvn.transform(new (gvn.C) MaxINode(a, b));
assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match");
return res;
} else {
Node* cmp = gvn.transform(new (gvn.C) CmpLNode(a, b));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveLNode(bol, a, b, t->is_long()));
}
} else {
if (is_int) {
Node* res = gvn.transform(new (gvn.C) MinINode(a, b));
assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match");
return res;
} else {
Node* cmp = gvn.transform(new (gvn.C) CmpLNode(b, a));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveLNode(bol, a, b, t->is_long()));
}
}
} else {
if (is_max) {
if (is_int) {
Node* cmp = gvn.transform(new (gvn.C) CmpUNode(a, b));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveINode(bol, a, b, t->is_int()));
} else {
Node* cmp = gvn.transform(new (gvn.C) CmpULNode(a, b));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveLNode(bol, a, b, t->is_long()));
}
} else {
if (is_int) {
Node* cmp = gvn.transform(new (gvn.C) CmpUNode(b, a));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveINode(bol, a, b, t->is_int()));
} else {
Node* cmp = gvn.transform(new (gvn.C) CmpULNode(b, a));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveLNode(bol, a, b, t->is_long()));
}
}
}
}

Node* MaxNode::build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const Type* t, PhaseGVN& gvn) {
bool is_int = gvn.type(a)->isa_int();
assert(is_int || gvn.type(a)->isa_long(), "int or long inputs");
assert(is_int == (gvn.type(b)->isa_int() != NULL), "inconsistent inputs");
Node* zero = NULL;
if (is_int) {
zero = gvn.intcon(0);
} else {
zero = gvn.longcon(0);
}
if (is_max) {
if (is_int) {
Node* cmp = gvn.transform(new (gvn.C) CmpINode(a, b));
Node* sub = gvn.transform(new (gvn.C) SubINode(a, b));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveINode(bol, sub, zero, t->is_int()));
} else {
Node* cmp = gvn.transform(new (gvn.C) CmpLNode(a, b));
Node* sub = gvn.transform(new (gvn.C) SubLNode(a, b));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveLNode(bol, sub, zero, t->is_long()));
}
} else {
if (is_int) {
Node* cmp = gvn.transform(new (gvn.C) CmpINode(b, a));
Node* sub = gvn.transform(new (gvn.C) SubINode(a, b));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveINode(bol, sub, zero, t->is_int()));
} else {
Node* cmp = gvn.transform(new (gvn.C) CmpLNode(b, a));
Node* sub = gvn.transform(new (gvn.C) SubLNode(a, b));
Node* bol = gvn.transform(new (gvn.C) BoolNode(cmp, BoolTest::lt));
return gvn.transform(new (gvn.C) CMoveLNode(bol, sub, zero, t->is_long()));
}
}
}

//=============================================================================
//------------------------------add_ring---------------------------------------
// Supplied function returns the sum of the inputs.
Expand Down
30 changes: 30 additions & 0 deletions hotspot/src/share/vm/opto/addnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,39 @@ class XorLNode : public AddNode {
// all the behavior of addition on a ring. Only new thing is that we allow
// 2 equal inputs to be equal.
class MaxNode : public AddNode {
private:
static Node* build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn);
static Node* build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const Type* t, PhaseGVN& gvn);

public:
MaxNode( Node *in1, Node *in2 ) : AddNode(in1,in2) {}
virtual int Opcode() const = 0;

static Node* unsigned_max(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
return build_min_max(a, b, true, true, t, gvn);
}

static Node* unsigned_min(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
return build_min_max(a, b, false, true, t, gvn);
}

static Node* signed_max(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
return build_min_max(a, b, true, false, t, gvn);
}

static Node* signed_min(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
return build_min_max(a, b, false, false, t, gvn);
}

// max(a-b, 0)
static Node* max_diff_with_zero(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
return build_min_max_diff_with_zero(a, b, true, t, gvn);
}

// min(a-b, 0)
static Node* min_diff_with_zero(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
return build_min_max_diff_with_zero(a, b, false, t, gvn);
}
};

//------------------------------MaxINode---------------------------------------
Expand Down
42 changes: 30 additions & 12 deletions hotspot/src/share/vm/opto/loopTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1544,22 +1544,40 @@ Node* PhaseIdealLoop::adjust_limit(bool is_positive_stride, Node* scale, Node* o
register_new_node(limit, pre_ctrl);
}

// Clamp the limit to handle integer under-/overflows.
// Clamp the limit to handle integer under-/overflows by using long values.
// We only convert the limit back to int when we handled under-/overflows.
// Note that all values are longs in the following computations.
// When reducing the limit, clamp to [min_jint, old_limit]:
// MIN(old_limit, MAX(limit, min_jint))
// INT(MINL(old_limit, MAXL(limit, min_jint)))
// - integer underflow of limit: MAXL chooses min_jint.
// - integer overflow of limit: MINL chooses old_limit (<= MAX_INT < limit)
// When increasing the limit, clamp to [old_limit, max_jint]:
// MAX(old_limit, MIN(limit, max_jint))
Node* cmp = new (C) CmpLNode(limit, _igvn.longcon(is_positive_stride ? min_jint : max_jint));
// INT(MAXL(old_limit, MINL(limit, max_jint)))
// - integer overflow of limit: MINL chooses max_jint.
// - integer underflow of limit: MAXL chooses old_limit (>= MIN_INT > limit)
// INT() is finally converting the limit back to an integer value.

// We use CMove nodes to implement long versions of min/max (MINL/MAXL).
// We use helper methods for inner MINL/MAXL which return CMoveL nodes to keep a long value for the outer MINL/MAXL comparison:
Node* inner_result_long;
if (is_positive_stride) {
inner_result_long = MaxNode::signed_max(limit, _igvn.longcon(min_jint), TypeLong::LONG, _igvn);
} else {
inner_result_long = MaxNode::signed_min(limit, _igvn.longcon(max_jint), TypeLong::LONG, _igvn);
}
set_subtree_ctrl(inner_result_long);

// Outer MINL/MAXL:
// The comparison is done with long values but the result is the converted back to int by using CmovI.
Node* old_limit_long = new (C) ConvI2LNode(old_limit);
register_new_node(old_limit_long, pre_ctrl);
Node* cmp = new (C) CmpLNode(old_limit_long, limit);
register_new_node(cmp, pre_ctrl);
Node* bol = new (C) BoolNode(cmp, is_positive_stride ? BoolTest::lt : BoolTest::gt);
Node* bol = new (C) BoolNode(cmp, is_positive_stride ? BoolTest::gt : BoolTest::lt);
register_new_node(bol, pre_ctrl);
limit = new (C) ConvL2INode(limit);
register_new_node(limit, pre_ctrl);
limit = new (C) CMoveINode(bol, limit, _igvn.intcon(is_positive_stride ? min_jint : max_jint), TypeInt::INT);
register_new_node(limit, pre_ctrl);

limit = is_positive_stride ? (Node*)(new (C) MinINode(old_limit, limit))
: (Node*)(new (C) MaxINode(old_limit, limit));
Node* inner_result_int = new (C) ConvL2INode(inner_result_long); // Could under-/overflow but that's fine as comparison was done with CmpL
register_new_node(inner_result_int, pre_ctrl);
limit = new (C) CMoveINode(bol, old_limit, inner_result_int, TypeInt::INT);
register_new_node(limit, pre_ctrl);
return limit;
}
Expand Down
158 changes: 158 additions & 0 deletions hotspot/test/compiler/rangechecks/TestRangeCheckLimits.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. 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 8262017
* @summary Dominator failure because ConvL2I node becomes TOP due to missing overflow/underflow handling in range check elimination
* in PhaseIdealLoop::add_constraint().
* @run main/othervm -Xcomp -XX:-TieredCompilation -XX:CompileCommand=compileonly,compiler.rangechecks.TestRangeCheckLimits::*
* compiler.rangechecks.TestRangeCheckLimits
*/

package compiler.rangechecks;

public class TestRangeCheckLimits {
static int a = 400;
static volatile int b;
static long lFld;
static int iFld;

public static void main(String[] k) {
// Test all cases in PhaseIdealLoop::add_constraint().
testPositiveCaseMainLoop();
testNegativeCaseMainLoop();
testPositiveCasePreLoop();
testNegativeCasePreLoop();
}

public static void testPositiveCaseMainLoop() {
int e, f, g = 0, h[] = new int[a];
double i[] = new double[a];
long j = 9;
Helper.init(h, 3);
for (e = 5; e < 154; e++) {
for (f = 1; f < 169; f += 2) {
b = e;
}
i[1] = b;
for (g = 8; g < 168; g += 2) {
j = g - 5;
if (j > Integer.MAX_VALUE - 1) {
switch (3) {
case 3:
}
}
}
}
if (g != 168) {
throw new RuntimeException("fail");
}
lFld = j;
}


public static void testPositiveCasePreLoop() {
int e, f, g = 0, h[] = new int[a];
double i[] = new double[a];
long j = 9;
Helper.init(h, 3);
for (e = 5; e < 154; e++) {
for (f = 1; f < 169; f += 2) {
b = e;
}
i[1] = b;
for (g = 8; g < 168; g += 2) {
j = g + 5;
if (j > 180) {
switch (3) {
case 3:
}
}
}
}
if (g != 168) {
throw new RuntimeException("fail");
}
lFld = j;
}

public static void testNegativeCaseMainLoop() {
int e, f, g = 0, h[] = new int[a];
double i[] = new double[a];
long j = 9;
Helper.init(h, 3);
for (e = 5; e < 154; e++) {
for (f = 1; f < 169; f += 2) {
b = e;
}
i[1] = b;
for (g = 8; g < 168; g += 2) {
j = g;
if (j < 5) {
switch (3) {
case 3:
}
}
}
}
if (g != 168) {
throw new RuntimeException("fail");
}
lFld = j;
}


public static void testNegativeCasePreLoop() {
int e, f, g = 0, h[] = new int[a];
double i[] = new double[a];
long j = 9;
Helper.init(h, 3);
for (e = 5; e < 154; e++) {
for (f = 1; f < 169; f += 2) {
b = e;
}
i[1] = b;
for (g = 168; g > 8; g -= 2) {
j = g - 5;
if (j > Integer.MAX_VALUE - 1) {
switch (3) {
case 3:
}
}
}
}
if (g != 8) {
throw new RuntimeException("fail");
}
lFld = j;
}
}

class Helper {
public static void init(int[] a, int seed) {
for (int j = 0; j < a.length; j++) {
a[j] = (j % 2 == 0) ? seed + j : seed - j;
}
}
}

1 comment on commit 385e34d

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.