changeset 8969:b9246bd13eeb

8135069: C2 replaces range checks by unsigned comparison with -1 Summary: i < 0 || i > -1 wrongly folded as i >u -1 Reviewed-by: kvn
author roland
date Fri, 11 Sep 2015 16:56:56 +0200
parents 486680e6ed5e
children bfb61f868681
files src/share/vm/opto/ifnode.cpp test/compiler/rangechecks/TestBadFoldCompare.java
diffstat 2 files changed, 94 insertions(+), 12 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/opto/ifnode.cpp	Mon Sep 14 09:11:03 2015 +0000
+++ b/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/test/compiler/rangechecks/TestBadFoldCompare.java	Mon Sep 14 09:11:03 2015 +0000
+++ b/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");
         }