8262017: C2: assert(n != __null) failed: Bad immediate dominator info.

Reviewed-by: roland
Backport-of: 2db9005

Author: TheRealMDoerr

src/hotspot/share/opto/addnode.cpp

@@ -29,6 +29,7 @@
 #include "opto/cfgnode.hpp"
 #include "opto/connode.hpp"
 #include "opto/machnode.hpp"
+#include "opto/movenode.hpp"
 #include "opto/mulnode.hpp"
 #include "opto/phaseX.hpp"
 #include "opto/subnode.hpp"
@@ -832,6 +833,95 @@ const Type *XorLNode::add_ring( const Type *t0, const Type *t1 ) const {
   return TypeLong::make( r0->get_con() ^ r1->get_con() );
 }
 
+
+Node* MaxNode::build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn) {
+  bool is_int = gvn.type(a)->isa_int();
+  assert(is_int || gvn.type(a)->isa_long(), "int or long inputs");
+  assert(is_int == (gvn.type(b)->isa_int() != NULL), "inconsistent inputs");
+  if (!is_unsigned) {
+    if (is_max) {
+      if (is_int) {
+        Node* res =  gvn.transform(new MaxINode(a, b));
+        assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match");
+        return res;
+      } else {
+        Node* cmp = gvn.transform(new CmpLNode(a, b));
+        Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+        return gvn.transform(new CMoveLNode(bol, a, b, t->is_long()));
+      }
+    } else {
+      if (is_int) {
+        Node* res =  gvn.transform(new MinINode(a, b));
+        assert(gvn.type(res)->is_int()->_lo >= t->is_int()->_lo && gvn.type(res)->is_int()->_hi <= t->is_int()->_hi, "type doesn't match");
+        return res;
+      } else {
+        Node* cmp = gvn.transform(new CmpLNode(b, a));
+        Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+        return gvn.transform(new CMoveLNode(bol, a, b, t->is_long()));
+      }
+    }
+  } else {
+    if (is_max) {
+      if (is_int) {
+        Node* cmp = gvn.transform(new CmpUNode(a, b));
+        Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+        return gvn.transform(new CMoveINode(bol, a, b, t->is_int()));
+      } else {
+        Node* cmp = gvn.transform(new CmpULNode(a, b));
+        Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+        return gvn.transform(new CMoveLNode(bol, a, b, t->is_long()));
+      }
+    } else {
+      if (is_int) {
+        Node* cmp = gvn.transform(new CmpUNode(b, a));
+        Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+        return gvn.transform(new CMoveINode(bol, a, b, t->is_int()));
+      } else {
+        Node* cmp = gvn.transform(new CmpULNode(b, a));
+        Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+        return gvn.transform(new CMoveLNode(bol, a, b, t->is_long()));
+      }
+    }
+  }
+}
+
+Node* MaxNode::build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const Type* t, PhaseGVN& gvn) {
+  bool is_int = gvn.type(a)->isa_int();
+  assert(is_int || gvn.type(a)->isa_long(), "int or long inputs");
+  assert(is_int == (gvn.type(b)->isa_int() != NULL), "inconsistent inputs");
+  Node* zero = NULL;
+  if (is_int) {
+    zero = gvn.intcon(0);
+  } else {
+    zero = gvn.longcon(0);
+  }
+  if (is_max) {
+    if (is_int) {
+      Node* cmp = gvn.transform(new CmpINode(a, b));
+      Node* sub = gvn.transform(new SubINode(a, b));
+      Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+      return gvn.transform(new CMoveINode(bol, sub, zero, t->is_int()));
+    } else {
+      Node* cmp = gvn.transform(new CmpLNode(a, b));
+      Node* sub = gvn.transform(new SubLNode(a, b));
+      Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+      return gvn.transform(new CMoveLNode(bol, sub, zero, t->is_long()));
+    }
+  } else {
+    if (is_int) {
+      Node* cmp = gvn.transform(new CmpINode(b, a));
+      Node* sub = gvn.transform(new SubINode(a, b));
+      Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+      return gvn.transform(new CMoveINode(bol, sub, zero, t->is_int()));
+    } else {
+      Node* cmp = gvn.transform(new CmpLNode(b, a));
+      Node* sub = gvn.transform(new SubLNode(a, b));
+      Node* bol = gvn.transform(new BoolNode(cmp, BoolTest::lt));
+      return gvn.transform(new CMoveLNode(bol, sub, zero, t->is_long()));
+    }
+  }
+}
+
 //=============================================================================
 //------------------------------add_ring---------------------------------------
 // Supplied function returns the sum of the inputs.

src/hotspot/share/opto/addnode.hpp

@@ -217,9 +217,39 @@ class XorLNode : public AddNode {
 // all the behavior of addition on a ring.  Only new thing is that we allow
 // 2 equal inputs to be equal.
 class MaxNode : public AddNode {
+private:
+  static Node* build_min_max(Node* a, Node* b, bool is_max, bool is_unsigned, const Type* t, PhaseGVN& gvn);
+  static Node* build_min_max_diff_with_zero(Node* a, Node* b, bool is_max, const Type* t, PhaseGVN& gvn);
+
 public:
   MaxNode( Node *in1, Node *in2 ) : AddNode(in1,in2) {}
   virtual int Opcode() const = 0;
+
+  static Node* unsigned_max(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
+    return build_min_max(a, b, true, true, t, gvn);
+  }
+
+  static Node* unsigned_min(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
+    return build_min_max(a, b, false, true, t, gvn);
+  }
+
+  static Node* signed_max(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
+    return build_min_max(a, b, true, false, t, gvn);
+  }
+
+  static Node* signed_min(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
+    return build_min_max(a, b, false, false, t, gvn);
+  }
+
+  // max(a-b, 0)
+  static Node* max_diff_with_zero(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
+    return build_min_max_diff_with_zero(a, b, true, t, gvn);
+  }
+
+  // min(a-b, 0)
+  static Node* min_diff_with_zero(Node* a, Node* b, const Type* t, PhaseGVN& gvn) {
+    return build_min_max_diff_with_zero(a, b, false, t, gvn);
+  }
 };
 
 //------------------------------MaxINode---------------------------------------

src/hotspot/share/opto/loopTransform.cpp

@@ -2178,22 +2178,40 @@ Node* PhaseIdealLoop::adjust_limit(bool is_positive_stride, Node* scale, Node* o
     register_new_node(limit, pre_ctrl);
   }
 
-  // Clamp the limit to handle integer under-/overflows.
+  // Clamp the limit to handle integer under-/overflows by using long values.
+  // We only convert the limit back to int when we handled under-/overflows.
+  // Note that all values are longs in the following computations.
   // When reducing the limit, clamp to [min_jint, old_limit]:
-  //   MIN(old_limit, MAX(limit, min_jint))
+  //   INT(MINL(old_limit, MAXL(limit, min_jint)))
+  //   - integer underflow of limit: MAXL chooses min_jint.
+  //   - integer overflow of limit: MINL chooses old_limit (<= MAX_INT < limit)
   // When increasing the limit, clamp to [old_limit, max_jint]:
-  //   MAX(old_limit, MIN(limit, max_jint))
-  Node* cmp = new CmpLNode(limit, _igvn.longcon(is_positive_stride ? min_jint : max_jint));
+  //   INT(MAXL(old_limit, MINL(limit, max_jint)))
+  //   - integer overflow of limit: MINL chooses max_jint.
+  //   - integer underflow of limit: MAXL chooses old_limit (>= MIN_INT > limit)
+  // INT() is finally converting the limit back to an integer value.
+
+  // We use CMove nodes to implement long versions of min/max (MINL/MAXL).
+  // We use helper methods for inner MINL/MAXL which return CMoveL nodes to keep a long value for the outer MINL/MAXL comparison:
+  Node* inner_result_long;
+  if (is_positive_stride) {
+    inner_result_long = MaxNode::signed_max(limit, _igvn.longcon(min_jint), TypeLong::LONG, _igvn);
+  } else {
+    inner_result_long = MaxNode::signed_min(limit, _igvn.longcon(max_jint), TypeLong::LONG, _igvn);
+  }
+  set_subtree_ctrl(inner_result_long);
+
+  // Outer MINL/MAXL:
+  // The comparison is done with long values but the result is the converted back to int by using CmovI.
+  Node* old_limit_long = new ConvI2LNode(old_limit);
+  register_new_node(old_limit_long, pre_ctrl);
+  Node* cmp = new CmpLNode(old_limit_long, limit);
   register_new_node(cmp, pre_ctrl);
-  Node* bol = new BoolNode(cmp, is_positive_stride ? BoolTest::lt : BoolTest::gt);
+  Node* bol = new BoolNode(cmp, is_positive_stride ? BoolTest::gt : BoolTest::lt);
   register_new_node(bol, pre_ctrl);
-  limit = new ConvL2INode(limit);
-  register_new_node(limit, pre_ctrl);
-  limit = new CMoveINode(bol, limit, _igvn.intcon(is_positive_stride ? min_jint : max_jint), TypeInt::INT);
-  register_new_node(limit, pre_ctrl);
-
-  limit = is_positive_stride ? (Node*)(new MinINode(old_limit, limit))
-                             : (Node*)(new MaxINode(old_limit, limit));
+  Node* inner_result_int = new ConvL2INode(inner_result_long); // Could under-/overflow but that's fine as comparison was done with CmpL
+  register_new_node(inner_result_int, pre_ctrl);
+  limit = new CMoveINode(bol, old_limit, inner_result_int, TypeInt::INT);
   register_new_node(limit, pre_ctrl);
   return limit;
 }

test/hotspot/jtreg/compiler/rangechecks/TestRangeCheckLimits.java

@@ -0,0 +1,158 @@
+/*
+ * Copyright (c) 2021, 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.
+ */
+
+/*
+ * @test
+ * @bug 8262017
+ * @summary Dominator failure because ConvL2I node becomes TOP due to missing overflow/underflow handling in range check elimination
+ *          in PhaseIdealLoop::add_constraint().
+ * @run main/othervm -Xcomp -XX:-TieredCompilation -XX:CompileCommand=compileonly,compiler.rangechecks.TestRangeCheckLimits::*
+ *                   compiler.rangechecks.TestRangeCheckLimits
+ */
+
+ package compiler.rangechecks;
+
+ public class TestRangeCheckLimits {
+    static int a = 400;
+    static volatile int b;
+    static long lFld;
+    static int iFld;
+
+    public static void main(String[] k) {
+        // Test all cases in PhaseIdealLoop::add_constraint().
+        testPositiveCaseMainLoop();
+        testNegativeCaseMainLoop();
+        testPositiveCasePreLoop();
+        testNegativeCasePreLoop();
+    }
+
+    public static void testPositiveCaseMainLoop() {
+        int e, f, g = 0, h[] = new int[a];
+        double i[] = new double[a];
+        long j = 9;
+        Helper.init(h, 3);
+        for (e = 5; e < 154; e++) {
+            for (f = 1; f < 169; f += 2) {
+                b = e;
+            }
+            i[1] = b;
+            for (g = 8; g < 168; g += 2) {
+                j = g - 5;
+                if (j > Integer.MAX_VALUE - 1) {
+                    switch (3) {
+                        case 3:
+                    }
+                }
+            }
+        }
+        if (g != 168) {
+            throw new RuntimeException("fail");
+        }
+        lFld = j;
+    }
+
+
+    public static void testPositiveCasePreLoop() {
+        int e, f, g = 0, h[] = new int[a];
+        double i[] = new double[a];
+        long j = 9;
+        Helper.init(h, 3);
+        for (e = 5; e < 154; e++) {
+            for (f = 1; f < 169; f += 2) {
+                b = e;
+            }
+            i[1] = b;
+            for (g = 8; g < 168; g += 2) {
+                j = g + 5;
+                if (j > 180) {
+                    switch (3) {
+                        case 3:
+                    }
+                }
+            }
+        }
+        if (g != 168) {
+            throw new RuntimeException("fail");
+        }
+        lFld = j;
+    }
+
+    public static void testNegativeCaseMainLoop() {
+        int e, f, g = 0, h[] = new int[a];
+        double i[] = new double[a];
+        long j = 9;
+        Helper.init(h, 3);
+        for (e = 5; e < 154; e++) {
+            for (f = 1; f < 169; f += 2) {
+                b = e;
+            }
+            i[1] = b;
+            for (g = 8; g < 168; g += 2) {
+                j = g;
+                if (j < 5) {
+                    switch (3) {
+                        case 3:
+                    }
+                }
+            }
+        }
+        if (g != 168) {
+            throw new RuntimeException("fail");
+        }
+        lFld = j;
+    }
+
+
+    public static void testNegativeCasePreLoop() {
+        int e, f, g = 0, h[] = new int[a];
+        double i[] = new double[a];
+        long j = 9;
+        Helper.init(h, 3);
+        for (e = 5; e < 154; e++) {
+            for (f = 1; f < 169; f += 2) {
+                b = e;
+            }
+            i[1] = b;
+            for (g = 168; g > 8; g -= 2) {
+                j = g - 5;
+                if (j > Integer.MAX_VALUE - 1) {
+                    switch (3) {
+                        case 3:
+                    }
+                }
+            }
+        }
+        if (g != 8) {
+            throw new RuntimeException("fail");
+        }
+        lFld = j;
+    }
+}
+
+class Helper {
+    public static void init(int[] a, int seed) {
+        for (int j = 0; j < a.length; j++) {
+            a[j] = (j % 2 == 0) ? seed + j : seed - j;
+        }
+    }
+}