8331090: Run Ideal_minmax before de-canonicalizing CMoves

Reviewed-by: thartmann, epeter

Author: jaskarth

src/hotspot/share/opto/movenode.cpp

@@ -91,17 +91,20 @@ Node *CMoveNode::Ideal(PhaseGVN *phase, bool can_reshape) {
       phase->type(in(IfTrue))    == Type::TOP) {
     return nullptr;
   }
-  // Canonicalize the node by moving constants to the right input.
-  if (in(Condition)->is_Bool() && phase->type(in(IfFalse))->singleton() && !phase->type(in(IfTrue))->singleton()) {
-    BoolNode* b = in(Condition)->as_Bool()->negate(phase);
-    return make(in(Control), phase->transform(b), in(IfTrue), in(IfFalse), _type);
-  }
 
+  // Check for Min/Max patterns. This is called before constants are pushed to the right input, as that transform can
+  // make BoolTests non-canonical.
   Node* minmax = Ideal_minmax(phase, this);
   if (minmax != nullptr) {
     return minmax;
   }
 
+  // Canonicalize the node by moving constants to the right input.
+  if (in(Condition)->is_Bool() && phase->type(in(IfFalse))->singleton() && !phase->type(in(IfTrue))->singleton()) {
+    BoolNode* b = in(Condition)->as_Bool()->negate(phase);
+    return make(in(Control), phase->transform(b), in(IfTrue), in(IfFalse), _type);
+  }
+
   return nullptr;
 }
 

test/hotspot/jtreg/compiler/c2/irTests/TestIfMinMax.java

@@ -30,7 +30,7 @@
 
 /*
  * @test
- * @bug 8324655 8329797
+ * @bug 8324655 8329797 8331090
  * @key randomness
  * @summary Test that if expressions are properly folded into min/max nodes
  * @requires os.arch != "riscv64"
@@ -505,7 +505,27 @@ public void checkTestMinLongVector(Object[] vals) {
         }
     }
 
-    @Run(test = { "testMinI1", "testMinI2", "testMaxI1", "testMaxI2", "testMinI1E", "testMinI2E", "testMaxI1E", "testMaxI2E" })
+    @Test
+    @IR(failOn = { IRNode.IF }, counts = { IRNode.MIN_I, "1" })
+    public int testMinIConst(int a) {
+        if (a > 65535) {
+            a = 65535;
+        }
+
+        return a;
+    }
+
+    @Test
+    @IR(phase = { CompilePhase.BEFORE_MACRO_EXPANSION }, failOn = { IRNode.IF }, counts = { IRNode.MIN_L, "1" })
+    public long testMinLConst(long a) {
+        if (a > 65535) {
+            a = 65535;
+        }
+
+        return a;
+    }
+
+    @Run(test = { "testMinI1", "testMinI2", "testMaxI1", "testMaxI2", "testMinI1E", "testMinI2E", "testMaxI1E", "testMaxI2E", "testMinIConst" })
     public void runTestIntegers() {
         testIntegers(10, 20);
         testIntegers(20, 10);
@@ -526,9 +546,12 @@ public void testIntegers(int a, int b) {
         Asserts.assertEQ(a >= b ? b : a, testMinI2E(a, b));
         Asserts.assertEQ(a >= b ? a : b, testMaxI1E(a, b));
         Asserts.assertEQ(a <= b ? b : a, testMaxI2E(a, b));
+
+        Asserts.assertEQ(a > 65535 ? 65535 : a, testMinIConst(a));
+        Asserts.assertEQ(b > 65535 ? 65535 : b, testMinIConst(b));
     }
 
-    @Run(test = { "testMinL1", "testMinL2", "testMaxL1", "testMaxL2", "testMinL1E", "testMinL2E", "testMaxL1E", "testMaxL2E" })
+    @Run(test = { "testMinL1", "testMinL2", "testMaxL1", "testMaxL2", "testMinL1E", "testMinL2E", "testMaxL1E", "testMaxL2E", "testMinLConst" })
     public void runTestLongs() {
         testLongs(10, 20);
         testLongs(20, 10);
@@ -551,5 +574,8 @@ public void testLongs(long a, long b) {
         Asserts.assertEQ(a >= b ? b : a, testMinL2E(a, b));
         Asserts.assertEQ(a >= b ? a : b, testMaxL1E(a, b));
         Asserts.assertEQ(a <= b ? b : a, testMaxL2E(a, b));
+
+        Asserts.assertEQ(a > 65535L ? 65535L : a, testMinLConst(a));
+        Asserts.assertEQ(b > 65535L ? 65535L : b, testMinLConst(b));
     }
 }