8312547: Max/Min nodes Value implementation could be improved

Reviewed-by: thartmann, kvn

Author: Quan Anh Mai

src/hotspot/share/opto/addnode.cpp

@@ -216,22 +216,19 @@ Node *AddNode::Ideal(PhaseGVN *phase, bool can_reshape) {
 // the other input's symbols.
 const Type* AddNode::Value(PhaseGVN* phase) const {
   // Either input is TOP ==> the result is TOP
-  const Type *t1 = phase->type( in(1) );
-  const Type *t2 = phase->type( in(2) );
-  if( t1 == Type::TOP ) return Type::TOP;
-  if( t2 == Type::TOP ) return Type::TOP;
-
-  // Either input is BOTTOM ==> the result is the local BOTTOM
-  const Type *bot = bottom_type();
-  if( (t1 == bot) || (t2 == bot) ||
-      (t1 == Type::BOTTOM) || (t2 == Type::BOTTOM) )
-    return bot;
+  const Type* t1 = phase->type(in(1));
+  const Type* t2 = phase->type(in(2));
+  if (t1 == Type::TOP || t2 == Type::TOP) {
+    return Type::TOP;
+  }
 
   // Check for an addition involving the additive identity
-  const Type *tadd = add_of_identity( t1, t2 );
-  if( tadd ) return tadd;
+  const Type* tadd = add_of_identity(t1, t2);
+  if (tadd != nullptr) {
+    return tadd;
+  }
 
-  return add_ring(t1,t2);               // Local flavor of type addition
+  return add_ring(t1, t2);               // Local flavor of type addition
 }
 
 //------------------------------add_identity-----------------------------------
@@ -513,7 +510,9 @@ const Type *AddFNode::add_of_identity( const Type *t1, const Type *t2 ) const {
 // This also type-checks the inputs for sanity.  Guaranteed never to
 // be passed a TOP or BOTTOM type, these are filtered out by pre-check.
 const Type *AddFNode::add_ring( const Type *t0, const Type *t1 ) const {
-  // We must be adding 2 float constants.
+  if (!t0->isa_float_constant() || !t1->isa_float_constant()) {
+    return bottom_type();
+  }
   return TypeF::make( t0->getf() + t1->getf() );
 }
 
@@ -544,7 +543,9 @@ const Type *AddDNode::add_of_identity( const Type *t1, const Type *t2 ) const {
 // This also type-checks the inputs for sanity.  Guaranteed never to
 // be passed a TOP or BOTTOM type, these are filtered out by pre-check.
 const Type *AddDNode::add_ring( const Type *t0, const Type *t1 ) const {
-  // We must be adding 2 double constants.
+  if (!t0->isa_double_constant() || !t1->isa_double_constant()) {
+    return bottom_type();
+  }
   return TypeD::make( t0->getd() + t1->getd() );
 }
 
@@ -1367,9 +1368,12 @@ Node* MinLNode::Ideal(PhaseGVN* phase, bool can_reshape) {
 }
 
 //------------------------------add_ring---------------------------------------
-const Type *MinFNode::add_ring( const Type *t0, const Type *t1 ) const {
-  const TypeF *r0 = t0->is_float_constant();
-  const TypeF *r1 = t1->is_float_constant();
+const Type* MinFNode::add_ring(const Type* t0, const Type* t1 ) const {
+  const TypeF* r0 = t0->isa_float_constant();
+  const TypeF* r1 = t1->isa_float_constant();
+  if (r0 == nullptr || r1 == nullptr) {
+    return bottom_type();
+  }
 
   if (r0->is_nan()) {
     return r0;
@@ -1389,9 +1393,12 @@ const Type *MinFNode::add_ring( const Type *t0, const Type *t1 ) const {
 }
 
 //------------------------------add_ring---------------------------------------
-const Type *MinDNode::add_ring( const Type *t0, const Type *t1 ) const {
-  const TypeD *r0 = t0->is_double_constant();
-  const TypeD *r1 = t1->is_double_constant();
+const Type* MinDNode::add_ring(const Type* t0, const Type* t1) const {
+  const TypeD* r0 = t0->isa_double_constant();
+  const TypeD* r1 = t1->isa_double_constant();
+  if (r0 == nullptr || r1 == nullptr) {
+    return bottom_type();
+  }
 
   if (r0->is_nan()) {
     return r0;
@@ -1411,9 +1418,12 @@ const Type *MinDNode::add_ring( const Type *t0, const Type *t1 ) const {
 }
 
 //------------------------------add_ring---------------------------------------
-const Type *MaxFNode::add_ring( const Type *t0, const Type *t1 ) const {
-  const TypeF *r0 = t0->is_float_constant();
-  const TypeF *r1 = t1->is_float_constant();
+const Type* MaxFNode::add_ring(const Type* t0, const Type* t1) const {
+  const TypeF* r0 = t0->isa_float_constant();
+  const TypeF* r1 = t1->isa_float_constant();
+  if (r0 == nullptr || r1 == nullptr) {
+    return bottom_type();
+  }
 
   if (r0->is_nan()) {
     return r0;
@@ -1433,9 +1443,12 @@ const Type *MaxFNode::add_ring( const Type *t0, const Type *t1 ) const {
 }
 
 //------------------------------add_ring---------------------------------------
-const Type *MaxDNode::add_ring( const Type *t0, const Type *t1 ) const {
-  const TypeD *r0 = t0->is_double_constant();
-  const TypeD *r1 = t1->is_double_constant();
+const Type* MaxDNode::add_ring(const Type* t0, const Type* t1) const {
+  const TypeD* r0 = t0->isa_double_constant();
+  const TypeD* r1 = t1->isa_double_constant();
+  if (r0 == nullptr || r1 == nullptr) {
+    return bottom_type();
+  }
 
   if (r0->is_nan()) {
     return r0;

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

@@ -28,7 +28,7 @@
 
 /*
  * @test
- * @bug 8290248
+ * @bug 8290248 8312547
  * @summary Test that Ideal transformations of MaxINode and MinINode are
  * being performed as expected.
  * @library /test/lib /
@@ -45,9 +45,11 @@ public static void main(String[] args) {
                  "testMax2L", "testMax2R",
                  "testMax2LNoLeftAdd",
                  "testMax3",
+                 "testMax4",
                  "testMin1",
                  "testMin2",
-                 "testMin3"})
+                 "testMin3",
+                 "testMin4"})
     public void runPositiveTests() {
         int a = RunInfo.getRandom().nextInt();
         int min = Integer.MIN_VALUE;
@@ -73,10 +75,12 @@ public void assertPositiveResult(int a) {
         Asserts.assertEQ(testMax2L(a)                                               , testMax2R(a));
         Asserts.assertEQ(Math.max(a >> 1, ((a >> 1) + 11))                          , testMax2LNoLeftAdd(a));
         Asserts.assertEQ(Math.max(a, a)                                             , testMax3(a));
+        Asserts.assertEQ(0                                                          , testMax4(a));
 
         Asserts.assertEQ(Math.min(((a >> 1) + 100), Math.min(((a >> 1) + 150), 200)), testMin1(a));
         Asserts.assertEQ(Math.min(((a >> 1) + 10), ((a >> 1) + 11))                 , testMin2(a));
         Asserts.assertEQ(Math.min(a, a)                                             , testMin3(a));
+        Asserts.assertEQ(0                                                          , testMin4(a));
     }
 
     // The transformations in test*1 and test*2 can happen only if the compiler has enough information
@@ -203,6 +207,18 @@ public int testMin3(int i) {
         return Math.min(i, i);
     }
 
+    @Test
+    @IR(failOn = {IRNode.MAX_I})
+    public int testMax4(int i) {
+        return Math.max(i, 0) < 0 ? 1 : 0;
+    }
+
+    @Test
+    @IR(failOn = {IRNode.MIN_I})
+    public int testMin4(int i) {
+        return Math.min(i, 0) > 0 ? 1 : 0;
+    }
+
     @Run(test = {"testTwoLevelsDifferentXY",
                  "testTwoLevelsNoLeftConstant",
                  "testTwoLevelsNoRightConstant",