8375653: C2: CmpUNode::sub is not monotonic

Reviewed-by: chagedorn, mchevalier

src/hotspot/share/opto/subnode.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2026, 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
@@ -745,71 +745,40 @@ const Type* CmpINode::Value(PhaseGVN* phase) const {
 
 // Simplify a CmpU (compare 2 integers) node, based on local information.
 // If both inputs are constants, compare them.
-const Type *CmpUNode::sub( const Type *t1, const Type *t2 ) const {
+const Type* CmpUNode::sub(const Type* t1, const Type* t2) const {
-  assert(!t1->isa_ptr(), "obsolete usage of CmpU");
+  const TypeInt* r0 = t1->is_int();
+  const TypeInt* r1 = t2->is_int();
 
-  // comparing two unsigned ints
-  const TypeInt *r0 = t1->is_int();   // Handy access
-  const TypeInt *r1 = t2->is_int();
-
-  // Current installed version
-  // Compare ranges for non-overlap
-  juint lo0 = r0->_lo;
-  juint hi0 = r0->_hi;
-  juint lo1 = r1->_lo;
-  juint hi1 = r1->_hi;
-
-  // If either one has both negative and positive values,
-  // it therefore contains both 0 and -1, and since [0..-1] is the
-  // full unsigned range, the type must act as an unsigned bottom.
-  bool bot0 = ((jint)(lo0 ^ hi0) < 0);
-  bool bot1 = ((jint)(lo1 ^ hi1) < 0);
-
-  if (bot0 || bot1) {
-    // All unsigned values are LE -1 and GE 0.
-    if (lo0 == 0 && hi0 == 0) {
-      return TypeInt::CC_LE;            //   0 <= bot
-    } else if ((jint)lo0 == -1 && (jint)hi0 == -1) {
-      return TypeInt::CC_GE;            // -1 >= bot
-    } else if (lo1 == 0 && hi1 == 0) {
-      return TypeInt::CC_GE;            // bot >= 0
-    } else if ((jint)lo1 == -1 && (jint)hi1 == -1) {
-      return TypeInt::CC_LE;            // bot <= -1
-    }
-  } else {
-    // We can use ranges of the form [lo..hi] if signs are the same.
-    assert(lo0 <= hi0 && lo1 <= hi1, "unsigned ranges are valid");
-    // results are reversed, '-' > '+' for unsigned compare
-    if (hi0 < lo1) {
-      return TypeInt::CC_LT;            // smaller
-    } else if (lo0 > hi1) {
-      return TypeInt::CC_GT;            // greater
-    } else if (hi0 == lo1 && lo0 == hi1) {
-      return TypeInt::CC_EQ;            // Equal results
-    } else if (lo0 >= hi1) {
-      return TypeInt::CC_GE;
-    } else if (hi0 <= lo1) {
-      // Check for special case in Hashtable::get.  (See below.)
-      if ((jint)lo0 >= 0 && (jint)lo1 >= 0 && is_index_range_check())
-        return TypeInt::CC_LT;
-      return TypeInt::CC_LE;
-    }
-  }
   // Check for special case in Hashtable::get - the hash index is
   // mod'ed to the table size so the following range check is useless.
   // Check for: (X Mod Y) CmpU Y, where the mod result and Y both have
   // to be positive.
   // (This is a gross hack, since the sub method never
   // looks at the structure of the node in any other case.)
-  if ((jint)lo0 >= 0 && (jint)lo1 >= 0 && is_index_range_check())
+  if (r0->_lo >= 0 && r1->_lo >= 0 && is_index_range_check()) {
     return TypeInt::CC_LT;
+  }
+
+  if (r0->_uhi < r1->_ulo) {
+    return TypeInt::CC_LT;
+  } else if (r0->_ulo > r1->_uhi) {
+    return TypeInt::CC_GT;
+  } else if (r0->is_con() && r1->is_con()) {
+    // Since r0->_ulo == r0->_uhi == r0->get_con(), we only reach here if the constants are equal
+    assert(r0->get_con() == r1->get_con(), "must reach a previous branch otherwise");
+    return TypeInt::CC_EQ;
+  } else if (r0->_uhi == r1->_ulo) {
+    return TypeInt::CC_LE;
+  } else if (r0->_ulo == r1->_uhi) {
+    return TypeInt::CC_GE;
+  }
 
   const Type* joined = r0->join(r1);
   if (joined == Type::TOP) {
     return TypeInt::CC_NE;
   }
 
-  return TypeInt::CC;                   // else use worst case results
+  return TypeInt::CC;
 }
 
 const Type* CmpUNode::Value(PhaseGVN* phase) const {
@@ -963,51 +932,21 @@ const Type *CmpLNode::sub( const Type *t1, const Type *t2 ) const {
 // Simplify a CmpUL (compare 2 unsigned longs) node, based on local information.
 // If both inputs are constants, compare them.
 const Type* CmpULNode::sub(const Type* t1, const Type* t2) const {
-  assert(!t1->isa_ptr(), "obsolete usage of CmpUL");
+  const TypeLong* r0 = t1->is_long();
-
-  // comparing two unsigned longs
-  const TypeLong* r0 = t1->is_long();   // Handy access
   const TypeLong* r1 = t2->is_long();
 
-  // Current installed version
+  if (r0->_uhi < r1->_ulo) {
-  // Compare ranges for non-overlap
+    return TypeInt::CC_LT;
-  julong lo0 = r0->_lo;
+  } else if (r0->_ulo > r1->_uhi) {
-  julong hi0 = r0->_hi;
+    return TypeInt::CC_GT;
-  julong lo1 = r1->_lo;
+  } else if (r0->is_con() && r1->is_con()) {
-  julong hi1 = r1->_hi;
+    // Since r0->_ulo == r0->_uhi == r0->get_con(), we only reach here if the constants are equal
-
+    assert(r0->get_con() == r1->get_con(), "must reach a previous branch otherwise");
-  // If either one has both negative and positive values,
+    return TypeInt::CC_EQ;
-  // it therefore contains both 0 and -1, and since [0..-1] is the
+  } else if (r0->_uhi == r1->_ulo) {
-  // full unsigned range, the type must act as an unsigned bottom.
+    return TypeInt::CC_LE;
-  bool bot0 = ((jlong)(lo0 ^ hi0) < 0);
+  } else if (r0->_ulo == r1->_uhi) {
-  bool bot1 = ((jlong)(lo1 ^ hi1) < 0);
+    return TypeInt::CC_GE;
-
-  if (bot0 || bot1) {
-    // All unsigned values are LE -1 and GE 0.
-    if (lo0 == 0 && hi0 == 0) {
-      return TypeInt::CC_LE;            //   0 <= bot
-    } else if ((jlong)lo0 == -1 && (jlong)hi0 == -1) {
-      return TypeInt::CC_GE;            // -1 >= bot
-    } else if (lo1 == 0 && hi1 == 0) {
-      return TypeInt::CC_GE;            // bot >= 0
-    } else if ((jlong)lo1 == -1 && (jlong)hi1 == -1) {
-      return TypeInt::CC_LE;            // bot <= -1
-    }
-  } else {
-    // We can use ranges of the form [lo..hi] if signs are the same.
-    assert(lo0 <= hi0 && lo1 <= hi1, "unsigned ranges are valid");
-    // results are reversed, '-' > '+' for unsigned compare
-    if (hi0 < lo1) {
-      return TypeInt::CC_LT;            // smaller
-    } else if (lo0 > hi1) {
-      return TypeInt::CC_GT;            // greater
-    } else if (hi0 == lo1 && lo0 == hi1) {
-      return TypeInt::CC_EQ;            // Equal results
-    } else if (lo0 >= hi1) {
-      return TypeInt::CC_GE;
-    } else if (hi0 <= lo1) {
-      return TypeInt::CC_LE;
-    }
   }
 
   const Type* joined = r0->join(r1);
@@ -1015,7 +954,7 @@ const Type* CmpULNode::sub(const Type* t1, const Type* t2) const {
     return TypeInt::CC_NE;
   }
 
-  return TypeInt::CC;                   // else use worst case results
+  return TypeInt::CC;
 }
 
 //=============================================================================

test/hotspot/jtreg/compiler/c2/gvn/CmpUNodeValueTests.java

@@ -0,0 +1,209 @@
+/*
+ * Copyright (c) 2026, 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.
+ */
+package compiler.c2.gvn;
+
+import compiler.lib.generators.Generators;
+import compiler.lib.ir_framework.*;
+import jdk.test.lib.Asserts;
+
+/*
+ * @test
+ * @bug 8375653
+ * @summary Test that Value computations of CmpUNode are being performed as expected.
+ * @library /test/lib /
+ * @run driver ${test.main.class}
+ */
+public class CmpUNodeValueTests {
+    public static void main(String[] args) {
+        TestFramework.run();
+    }
+
+    @DontInline
+    public static int one() {
+        return 1;
+    }
+
+    // Move to a separate method to prevent javac folding the constant comparison
+    @ForceInline
+    public static int oneInline() {
+        return 1;
+    }
+
+    @Run(test = {"testIntControl", "testIntLT", "testIntLE", "testIntGT", "testIntGE", "testIntEQ", "testIntNE",
+                 "testLongControl", "testLongLT", "testLongLE", "testLongGT", "testLongGE", "testLongEQ", "testLongNE"})
+    public void run() {
+        var stream = Generators.G.longs();
+        long x = stream.next();
+        long y = stream.next();
+
+        Asserts.assertEQ(0, testIntControl(false, false));
+        Asserts.assertEQ(0, testIntControl(false, true));
+        Asserts.assertEQ(0, testIntControl(true, false));
+        Asserts.assertEQ(1, testIntControl(true, true));
+        Asserts.assertEQ(0, testIntLT((int) x));
+        Asserts.assertEQ(0, testIntLE(false, false));
+        Asserts.assertEQ(0, testIntLE(false, true));
+        Asserts.assertEQ(0, testIntLE(true, false));
+        Asserts.assertEQ(0, testIntLE(true, true));
+        Asserts.assertEQ(0, testIntGT(false, false));
+        Asserts.assertEQ(0, testIntGT(false, true));
+        Asserts.assertEQ(0, testIntGT(true, false));
+        Asserts.assertEQ(0, testIntGT(true, true));
+        Asserts.assertEQ(0, testIntGE((int) x, (int) y));
+        Asserts.assertEQ(0, testIntEQ());
+        Asserts.assertEQ(0, testIntNE(false, false));
+        Asserts.assertEQ(0, testIntNE(false, true));
+        Asserts.assertEQ(0, testIntNE(true, false));
+        Asserts.assertEQ(0, testIntNE(true, true));
+
+        Asserts.assertEQ(0, testLongControl(false, false));
+        Asserts.assertEQ(0, testLongControl(false, true));
+        Asserts.assertEQ(0, testLongControl(true, false));
+        Asserts.assertEQ(1, testLongControl(true, true));
+        Asserts.assertEQ(0, testLongLT((int) x, false));
+        Asserts.assertEQ(0, testLongLT((int) x, true));
+        Asserts.assertEQ(0, testLongLE(false, false));
+        Asserts.assertEQ(0, testLongLE(false, true));
+        Asserts.assertEQ(0, testLongLE(true, false));
+        Asserts.assertEQ(0, testLongLE(true, true));
+        Asserts.assertEQ(0, testLongGT(false, false));
+        Asserts.assertEQ(0, testLongGT(false, true));
+        Asserts.assertEQ(0, testLongGT(true, false));
+        Asserts.assertEQ(0, testLongGT(true, true));
+        Asserts.assertEQ(0, testLongGE((int) x, (int) y));
+        Asserts.assertEQ(0, testLongEQ());
+        Asserts.assertEQ(0, testLongNE(false, false));
+        Asserts.assertEQ(0, testLongNE(false, true));
+        Asserts.assertEQ(0, testLongNE(true, false));
+        Asserts.assertEQ(0, testLongNE(true, true));
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, counts = {IRNode.CMP_U, "1"})
+    int testIntControl(boolean b1, boolean b2) {
+        int v1 = b1 ? 1 : -1;
+        int v2 = b2 ? 1 : 0;
+        return Integer.compareUnsigned(v1, v2) > 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
+    int testIntLT(int x) {
+        int v1 = x & Integer.MAX_VALUE; // Unset the highest bit, make v1 non-negative
+        int v2 = Integer.MIN_VALUE;
+        return Integer.compareUnsigned(v1, v2) < 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
+    int testIntLE(boolean b1, boolean b2) {
+        int v1 = b1 ? 2 : 0;
+        int v2 = b2 ? -1 : 2;
+        return Integer.compareUnsigned(v1, v2) <= 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
+    int testIntGT(boolean b1, boolean b2) {
+        int v1 = b1 ? Integer.MIN_VALUE : Integer.MAX_VALUE;
+        int v2 = b2 ? 0 : 2;
+        return Integer.compareUnsigned(v1, v2) > 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
+    int testIntGE(int x, int y) {
+        int v1 = x | Integer.MIN_VALUE; // Set the highest bit, make v1 negative
+        int v2 = y & Integer.MAX_VALUE; // Unset the highest bit, make v2 non-negative
+        return Integer.compareUnsigned(v1, v2) >= 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
+    int testIntEQ() {
+        return Integer.compareUnsigned(oneInline(), 1) == 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_U, IRNode.CALL})
+    int testIntNE(boolean b1, boolean b2) {
+        int v1 = b1 ? 1 : -1;
+        int v2 = b2 ? 0 : 2;
+        return Integer.compareUnsigned(v1, v2) != 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, counts = {IRNode.CMP_UL, "1"})
+    int testLongControl(boolean b1, boolean b2) {
+        long v1 = b1 ? 1 : -1;
+        long v2 = b2 ? 1 : 0;
+        return Long.compareUnsigned(v1, v2) > 0 ? 0 : one();
+    }
+
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
+    int testLongLT(int x, boolean b2) {
+        long v1 = Integer.toUnsignedLong(x);
+        long v2 = Integer.MIN_VALUE;
+        return Long.compareUnsigned(v1, v2) < 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
+    int testLongLE(boolean b1, boolean b2) {
+        long v1 = b1 ? 2 : 0;
+        long v2 = b2 ? -1 : 2;
+        return Long.compareUnsigned(v1, v2) <= 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
+    int testLongGT(boolean b1, boolean b2) {
+        long v1 = b1 ? Long.MIN_VALUE : Long.MAX_VALUE;
+        long v2 = b2 ? 0 : 2;
+        return Long.compareUnsigned(v1, v2) > 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
+    int testLongGE(int x, int y) {
+        long v1 = x | Long.MIN_VALUE; // Set the highest bit, make v1 negative
+        long v2 = y & Long.MAX_VALUE; // Unset the highest bit, make v2 non-negative
+        return Long.compareUnsigned(v1, v2) >= 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
+    int testLongEQ() {
+        return Long.compareUnsigned(oneInline(), 1L) == 0 ? 0 : one();
+    }
+
+    @Test
+    @IR(applyIfPlatformOr = {"x64", "true", "aarch64", "true"}, failOn = {IRNode.CMP_UL, IRNode.CALL})
+    int testLongNE(boolean b1, boolean b2) {
+        long v1 = b1 ? 1 : -1;
+        long v2 = b2 ? 0 : 2;
+        return Long.compareUnsigned(v1, v2) != 0 ? 0 : one();
+    }
+}

test/hotspot/jtreg/compiler/ccp/TestCmpUMonotonicity.java

@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2026, 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.
+ */
+package compiler.ccp;
+
+/*
+ * @test
+ * @bug 8375653
+ * @summary Test that CmpUNode::sub conforms monotonicity
+ *
+ * @run main ${test.main.class}
+ * @run main/othervm -Xbatch -XX:CompileCommand=compileonly,${test.main.class}::test* ${test.main.class}
+ */
+public class TestCmpUMonotonicity {
+    public static void main(String[] args) {
+        for (int i = 0; i < 20000; i++) {
+            testInt(true, 1, 100, 2);
+            testInt(false, 1, 100, 2);
+            testLong(true, 1, 100, 2);
+            testLong(false, 1, 100, 2);
+        }
+    }
+
+    private static int testInt(boolean b, int start, int limit, int step) {
+        int v2 = b ? 1 : -1;
+        int v1 = 0;
+        for (int i = start; i < limit; i *= step) {
+            if (Integer.compareUnsigned(v1, v2) < 0) {
+                v1 = 2;
+            } else {
+                v1 = 0;
+            }
+        }
+        return v1;
+    }
+
+    private static long testLong(boolean b, int start, int limit, int step) {
+        long v2 = b ? 1 : -1;
+        long v1 = 0;
+        for (int i = start; i < limit; i *= step) {
+            if (Long.compareUnsigned(v1, v2) < 0) {
+                v1 = 2;
+            } else {
+                v1 = 0;
+            }
+        }
+        return v1;
+    }
+}