8135069: C2 replaces range checks by unsigned comparison with -1
Summary: i < 0 || i > -1 wrongly folded as i >u -1
Reviewed-by: kvn
--- a/hotspot/src/share/vm/opto/ifnode.cpp Mon Sep 14 09:11:03 2015 +0000
+++ b/hotspot/src/share/vm/opto/ifnode.cpp Fri Sep 11 16:56:56 2015 +0200
@@ -858,18 +858,29 @@
// this_bool = <=
// dom_bool = >= (proj = True) or dom_bool = < (proj = False)
// x in [a, b] on the fail (= True) projection, b+1 > a-1:
- // lo = a, hi = b, adjusted_lim = b-a, cond = <=u
+ // lo = a, hi = b, adjusted_lim = b-a+1, cond = <u
+ // lo = a, hi = b, adjusted_lim = b-a, cond = <=u doesn't work because b = a - 1 is possible, then b-a = -1
// dom_bool = > (proj = True) or dom_bool = <= (proj = False)
// x in ]a, b] on the fail (= True) projection b+1 > a:
// lo = a+1, hi = b, adjusted_lim = b-a, cond = <u
- // lo = a+1, hi = b, adjusted_lim = b-a-1, cond = <=u doesn't work because a = b is possible, then hi-lo = -1
+ // lo = a+1, hi = b, adjusted_lim = b-a-1, cond = <=u doesn't work because a = b is possible, then b-a-1 = -1
- if (lo_test == BoolTest::gt || lo_test == BoolTest::le) {
- if (hi_test == BoolTest::le) {
+ if (hi_test == BoolTest::lt) {
+ if (lo_test == BoolTest::gt || lo_test == BoolTest::le) {
+ lo = igvn->transform(new AddINode(lo, igvn->intcon(1)));
+ }
+ } else {
+ assert(hi_test == BoolTest::le, "bad test");
+ if (lo_test == BoolTest::ge || lo_test == BoolTest::lt) {
adjusted_lim = igvn->transform(new SubINode(hi, lo));
+ adjusted_lim = igvn->transform(new AddINode(adjusted_lim, igvn->intcon(1)));
+ cond = BoolTest::lt;
+ } else {
+ assert(lo_test == BoolTest::gt || lo_test == BoolTest::le, "bad test");
+ adjusted_lim = igvn->transform(new SubINode(hi, lo));
+ lo = igvn->transform(new AddINode(lo, igvn->intcon(1)));
cond = BoolTest::lt;
}
- lo = igvn->transform(new AddINode(lo, igvn->intcon(1)));
}
} else if (lo_type->_lo > hi_type->_hi && lo_type->_hi == max_jint && hi_type->_lo == min_jint) {
@@ -879,7 +890,8 @@
// lo = b, hi = a, adjusted_lim = a-b, cond = >=u
// dom_bool = <= (proj = True) or dom_bool = > (proj = False)
// x in [b, a] on the fail (= False) projection, a+1 > b-1:
- // lo = b, hi = a, adjusted_lim = a-b, cond = >u
+ // lo = b, hi = a, adjusted_lim = a-b+1, cond = >=u
+ // lo = b, hi = a, adjusted_lim = a-b, cond = >u doesn't work because a = b - 1 is possible, then b-a = -1
// this_bool = <=
// dom_bool = < (proj = True) or dom_bool = >= (proj = False)
// x in ]b, a[ on the fail (= False) projection, a > b:
@@ -887,7 +899,7 @@
// dom_bool = <= (proj = True) or dom_bool = > (proj = False)
// x in ]b, a] on the fail (= False) projection, a+1 > b:
// lo = b+1, hi = a, adjusted_lim = a-b, cond = >=u
- // lo = b+1, hi = a, adjusted_lim = a-b-1, cond = >u doesn't work because a = b is possible, then hi-lo = -1
+ // lo = b+1, hi = a, adjusted_lim = a-b-1, cond = >u doesn't work because a = b is possible, then b-a-1 = -1
swap(lo, hi);
swap(lo_type, hi_type);
@@ -900,14 +912,26 @@
cond = (hi_test == BoolTest::le || hi_test == BoolTest::gt) ? BoolTest::gt : BoolTest::ge;
- if (lo_test == BoolTest::le) {
- if (cond == BoolTest::gt) {
+ if (lo_test == BoolTest::lt) {
+ if (hi_test == BoolTest::lt || hi_test == BoolTest::ge) {
+ cond = BoolTest::ge;
+ } else {
+ assert(hi_test == BoolTest::le || hi_test == BoolTest::gt, "bad test");
adjusted_lim = igvn->transform(new SubINode(hi, lo));
+ adjusted_lim = igvn->transform(new AddINode(adjusted_lim, igvn->intcon(1)));
cond = BoolTest::ge;
}
- lo = igvn->transform(new AddINode(lo, igvn->intcon(1)));
+ } else if (lo_test == BoolTest::le) {
+ if (hi_test == BoolTest::lt || hi_test == BoolTest::ge) {
+ lo = igvn->transform(new AddINode(lo, igvn->intcon(1)));
+ cond = BoolTest::ge;
+ } else {
+ assert(hi_test == BoolTest::le || hi_test == BoolTest::gt, "bad test");
+ adjusted_lim = igvn->transform(new SubINode(hi, lo));
+ lo = igvn->transform(new AddINode(lo, igvn->intcon(1)));
+ cond = BoolTest::ge;
+ }
}
-
} else {
const TypeInt* failtype = filtered_int_type(igvn, n, proj);
if (failtype != NULL) {
--- a/hotspot/test/compiler/rangechecks/TestBadFoldCompare.java Mon Sep 14 09:11:03 2015 +0000
+++ b/hotspot/test/compiler/rangechecks/TestBadFoldCompare.java Fri Sep 11 16:56:56 2015 +0200
@@ -24,7 +24,8 @@
/*
* @test
* @bug 8085832
- * @summary x <= 0 || x > 0 wrongly folded as (x-1) >u -1
+ * @bug 8135069
+ * @summary x <= 0 || x > 0 wrongly folded as (x-1) >u -1 and x < 0 || x > -1 wrongly folded as x >u -1
* @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement TestBadFoldCompare
*/
@@ -58,6 +59,34 @@
helper2(i, 0, 0, flag);
}
+ static boolean test3_taken;
+
+ static void helper3(int i, int a, int b, boolean flag) {
+ if (flag) {
+ if (i < a || i > b - 1) {
+ test3_taken = true;
+ }
+ }
+ }
+
+ static void test3(int i, boolean flag) {
+ helper3(i, 0, 0, flag);
+ }
+
+ static boolean test4_taken;
+
+ static void helper4(int i, int a, int b, boolean flag) {
+ if (flag) {
+ if (i > b - 1 || i < a) {
+ test4_taken = true;
+ }
+ }
+ }
+
+ static void test4(int i, boolean flag) {
+ helper4(i, 0, 0, flag);
+ }
+
static public void main(String[] args) {
boolean success = true;
@@ -87,6 +116,35 @@
System.out.println("Test2 failed");
success = false;
}
+
+ for (int i = 0; i < 20000; i++) {
+ helper3(5, 0, 10, (i%2)==0);
+ helper3(-1, 0, 10, (i%2)==0);
+ helper3(15, 0, 10, (i%2)==0);
+ test3(0, false);
+ }
+ test3_taken = false;
+ test3(0, true);
+
+ if (!test3_taken) {
+ System.out.println("Test3 failed");
+ success = false;
+ }
+
+ for (int i = 0; i < 20000; i++) {
+ helper4(5, 0, 10, (i%2)==0);
+ helper4(-1, 0, 10, (i%2)==0);
+ helper4(15, 0, 10, (i%2)==0);
+ test4(0, false);
+ }
+ test4_taken = false;
+ test4(0, true);
+
+ if (!test4_taken) {
+ System.out.println("Test4 failed");
+ success = false;
+ }
+
if (!success) {
throw new RuntimeException("Some tests failed");
}