changeset 59294:1a7175456d29

8235452: Strip mined loop verification fails with assert(is_OuterStripMinedLoop()) failed: invalid node class Summary: Do not try to verify strip mining if the strip mined loop is malformed. Reviewed-by: roland, vlivanov
author thartmann
date Wed, 11 Dec 2019 10:42:44 +0100
parents dcf88e5c8c07
children 6cf6761c444e
files src/hotspot/share/opto/loopPredicate.cpp src/hotspot/share/opto/loopnode.cpp src/hotspot/share/opto/loopnode.hpp test/hotspot/jtreg/compiler/loopstripmining/TestDeadOuterStripMinedLoop.java
diffstat 4 files changed, 108 insertions(+), 5 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/opto/loopPredicate.cpp	Mon Dec 09 16:14:16 2019 +0100
+++ b/src/hotspot/share/opto/loopPredicate.cpp	Wed Dec 11 10:42:44 2019 +0100
@@ -1388,9 +1388,8 @@
     // an early loop exit. Try them with profile data.
     while (if_proj_list.size() > 0) {
       Node* proj = if_proj_list.pop();
-      float f = pf.to(proj);
       if (proj->as_Proj()->is_uncommon_trap_if_pattern(Deoptimization::Reason_none) &&
-          f * loop_trip_cnt >= 1) {
+          pf.to(proj) * loop_trip_cnt >= 1) {
         hoisted = loop_predication_impl_helper(loop, proj->as_Proj(), profile_predicate_proj, cl, zero, invar, Deoptimization::Reason_profile_predicate) | hoisted;
       }
     }
--- a/src/hotspot/share/opto/loopnode.cpp	Mon Dec 09 16:14:16 2019 +0100
+++ b/src/hotspot/share/opto/loopnode.cpp	Wed Dec 11 10:42:44 2019 +0100
@@ -944,8 +944,11 @@
   return RegionNode::Ideal(phase, can_reshape);
 }
 
+#ifdef ASSERT
 void LoopNode::verify_strip_mined(int expect_skeleton) const {
-#ifdef ASSERT
+  if (!is_valid_counted_loop()) {
+    return; // Skip malformed counted loop
+  }
   const OuterStripMinedLoopNode* outer = NULL;
   const CountedLoopNode* inner = NULL;
   if (is_strip_mined()) {
@@ -955,6 +958,7 @@
   } else if (is_OuterStripMinedLoop()) {
     outer = this->as_OuterStripMinedLoop();
     inner = outer->unique_ctrl_out()->as_CountedLoop();
+    assert(inner->is_valid_counted_loop(), "OuterStripMinedLoop should have been removed");
     assert(!is_strip_mined(), "outer loop shouldn't be marked strip mined");
   }
   if (inner != NULL || outer != NULL) {
@@ -1024,8 +1028,8 @@
     assert(sfpt->outcnt() == 1, "no data node");
     assert(outer_tail->outcnt() == 1 || !has_skeleton, "no data node");
   }
+}
 #endif
-}
 
 //=============================================================================
 //------------------------------Ideal------------------------------------------
--- a/src/hotspot/share/opto/loopnode.hpp	Mon Dec 09 16:14:16 2019 +0100
+++ b/src/hotspot/share/opto/loopnode.hpp	Wed Dec 11 10:42:44 2019 +0100
@@ -152,7 +152,7 @@
   virtual void dump_spec(outputStream *st) const;
 #endif
 
-  void verify_strip_mined(int expect_skeleton) const;
+  void verify_strip_mined(int expect_skeleton) const NOT_DEBUG_RETURN;
   virtual LoopNode* skip_strip_mined(int expect_skeleton = 1) { return this; }
   virtual IfTrueNode* outer_loop_tail() const { ShouldNotReachHere(); return NULL; }
   virtual OuterStripMinedLoopEndNode* outer_loop_end() const { ShouldNotReachHere(); return NULL; }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/loopstripmining/TestDeadOuterStripMinedLoop.java	Wed Dec 11 10:42:44 2019 +0100
@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2019, 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 8235452
+ * @library /test/lib /
+ * @summary Test loop strip mining verification with dying outer loop.
+ * @run main/othervm -Xbatch -XX:-TieredCompilation
+ *                   compiler.loopstripmining.TestDeadOuterStripMinedLoop
+ */
+
+package compiler.loopstripmining;
+
+import jdk.test.lib.Asserts;
+
+public class TestDeadOuterStripMinedLoop {
+
+    public static int test(boolean b) {
+        int res = 5000;
+        for (int i = 0; i < 42; i++) {
+            if (!b) {
+                break;
+            }
+            // Strip mined loop
+            while (--res > 0) {
+                if (res < 1) {
+                    throw new RuntimeException("Should not reach here!");
+                }
+            }
+
+            /*
+            After parsing:
+                Loop:
+                    res--;
+                    if (res <= 0) {
+                        goto End;
+                    }
+                    if (res < 1) {  <- Treated as loop exit check (res <= 0)
+                        UncommonTrap();
+                    }
+                goto Loop;
+                End:
+
+            Loop opts convert this into:
+                // Outer strip mined loop
+                do {
+                    // Strip mined loop
+                    do {
+                        res--;
+                        if (res <= 0) {
+                            goto End;
+                        }
+                    } while (res > 0);  <- Always false due to above res <= 0 check
+                    Safepoint()
+                } while (false);
+                UncommonTrap();
+                End:
+
+            The safepoint path dies, because the exit condition of the strip mined loop is always false:
+                // Strip mined loop
+                do {
+                    res--;
+                    if (res <= 0) {
+                        goto End;
+                    }
+                } while (true);
+                End:
+            */
+        }
+        return res;
+    }
+
+    public static void main(String[] args) {
+        for (int i = 0; i < 100; i++) {
+            Asserts.assertEQ(test(true), -41);
+        }
+        Asserts.assertEQ(test(false), 5000);
+    }
+}