From 4918f26cc615a59666f377ea9cb6de7a0d09c7ee Mon Sep 17 00:00:00 2001
From: Jasmine Karthikeyan <25208576+jaskarth@users.noreply.github.com>
Date: Sat, 22 Jun 2024 13:33:00 -0400
Subject: [PATCH] Move Min/Max opt before CMove de-canonicalization

---
 src/hotspot/share/opto/movenode.cpp           | 13 +++++---
 .../compiler/c2/irTests/TestIfMinMax.java     | 32 +++++++++++++++++--
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/src/hotspot/share/opto/movenode.cpp b/src/hotspot/share/opto/movenode.cpp
index eb0a914b99433..dc65afff26f0f 100644
--- a/src/hotspot/share/opto/movenode.cpp
+++ b/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;
 }
 
diff --git a/test/hotspot/jtreg/compiler/c2/irTests/TestIfMinMax.java b/test/hotspot/jtreg/compiler/c2/irTests/TestIfMinMax.java
index de0b41e73ea86..d2ced1af563b5 100644
--- a/test/hotspot/jtreg/compiler/c2/irTests/TestIfMinMax.java
+++ b/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));
     }
 }