changeset 50819:ff5d0ea58d9b

8192992: Test8007294.java failed: attempted to spill a non-spillable item Summary: Fix bugs in schedule_late that forces load to early Reviewed-by: kvn, thartmann
author neliasso
date Wed, 21 Mar 2018 16:03:12 +0100
parents 6ba3e32a9882
children a11c1cb542bb
files src/hotspot/share/opto/compile.cpp src/hotspot/share/opto/gcm.cpp src/hotspot/share/opto/phasetype.hpp test/hotspot/jtreg/compiler/c2/Test6843752.java
diffstat 4 files changed, 45 insertions(+), 39 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/opto/compile.cpp	Wed May 23 15:21:54 2018 +0200
+++ b/src/hotspot/share/opto/compile.cpp	Wed Mar 21 16:03:12 2018 +0100
@@ -2422,6 +2422,8 @@
     return;
   }
 
+  print_method(PHASE_MATCHING, 2);
+
   // Build a proper-looking CFG
   PhaseCFG cfg(node_arena(), root(), matcher);
   _cfg = &cfg;
--- a/src/hotspot/share/opto/gcm.cpp	Wed May 23 15:21:54 2018 +0200
+++ b/src/hotspot/share/opto/gcm.cpp	Wed Mar 21 16:03:12 2018 +0100
@@ -684,49 +684,51 @@
     assert(store_block != NULL, "unused killing projections skipped above");
 
     if (store->is_Phi()) {
-      // 'load' uses memory which is one (or more) of the Phi's inputs.
-      // It must be scheduled not before the Phi, but rather before
-      // each of the relevant Phi inputs.
-      //
-      // Instead of finding the LCA of all inputs to a Phi that match 'mem',
-      // we mark each corresponding predecessor block and do a combined
-      // hoisting operation later (raise_LCA_above_marks).
-      //
-      // Do not assert(store_block != early, "Phi merging memory after access")
-      // PhiNode may be at start of block 'early' with backedge to 'early'
-      DEBUG_ONLY(bool found_match = false);
-      for (uint j = PhiNode::Input, jmax = store->req(); j < jmax; j++) {
-        if (store->in(j) == mem) {   // Found matching input?
-          DEBUG_ONLY(found_match = true);
-          Block* pred_block = get_block_for_node(store_block->pred(j));
-          if (pred_block != early) {
-            // If any predecessor of the Phi matches the load's "early block",
-            // we do not need a precedence edge between the Phi and 'load'
-            // since the load will be forced into a block preceding the Phi.
-            pred_block->set_raise_LCA_mark(load_index);
-            assert(!LCA_orig->dominates(pred_block) ||
-                   early->dominates(pred_block), "early is high enough");
-            must_raise_LCA = true;
-          } else {
-            // anti-dependent upon PHI pinned below 'early', no edge needed
-            LCA = early;             // but can not schedule below 'early'
+      if (store->in(0)->is_Loop()) {
+        // 'load' uses memory which is one (or more) of the Phi's inputs.
+        // It must be scheduled not before the Phi, but rather before
+        // each of the relevant Phi inputs.
+        //
+        // Instead of finding the LCA of all inputs to a Phi that match 'mem',
+        // we mark each corresponding predecessor block and do a combined
+        // hoisting operation later (raise_LCA_above_marks).
+        //
+        // Do not assert(store_block != early, "Phi merging memory after access")
+        // PhiNode may be at start of block 'early' with backedge to 'early'
+        DEBUG_ONLY(bool found_match = false);
+        for (uint j = PhiNode::Input, jmax = store->req(); j < jmax; j++) {
+          if (store->in(j) == mem) {   // Found matching input?
+            DEBUG_ONLY(found_match = true);
+            Block* pred_block = get_block_for_node(store_block->pred(j));
+            if (pred_block != early) {
+              // If any predecessor of the Phi matches the load's "early block",
+              // we do not need a precedence edge between the Phi and 'load'
+              // since the load will be forced into a block preceding the Phi.
+              pred_block->set_raise_LCA_mark(load_index);
+              assert(!LCA_orig->dominates(pred_block) ||
+                     early->dominates(pred_block), "early is high enough");
+              must_raise_LCA = true;
+            } else {
+              // anti-dependent upon PHI pinned below 'early', no edge needed
+              LCA = early;             // but can not schedule below 'early'
+            }
           }
         }
-      }
-      assert(found_match, "no worklist bug");
+        assert(found_match, "no worklist bug");
 #ifdef TRACK_PHI_INPUTS
 #ifdef ASSERT
-      // This assert asks about correct handling of PhiNodes, which may not
-      // have all input edges directly from 'mem'. See BugId 4621264
-      int num_mem_inputs = phi_inputs.at_grow(store->_idx,0) + 1;
-      // Increment by exactly one even if there are multiple copies of 'mem'
-      // coming into the phi, because we will run this block several times
-      // if there are several copies of 'mem'.  (That's how DU iterators work.)
-      phi_inputs.at_put(store->_idx, num_mem_inputs);
-      assert(PhiNode::Input + num_mem_inputs < store->req(),
-             "Expect at least one phi input will not be from original memory state");
+        // This assert asks about correct handling of PhiNodes, which may not
+        // have all input edges directly from 'mem'. See BugId 4621264
+        int num_mem_inputs = phi_inputs.at_grow(store->_idx,0) + 1;
+        // Increment by exactly one even if there are multiple copies of 'mem'
+        // coming into the phi, because we will run this block several times
+        // if there are several copies of 'mem'.  (That's how DU iterators work.)
+        phi_inputs.at_put(store->_idx, num_mem_inputs);
+        assert(PhiNode::Input + num_mem_inputs < store->req(),
+               "Expect at least one phi input will not be from original memory state");
 #endif //ASSERT
 #endif //TRACK_PHI_INPUTS
+      }
     } else if (store_block != early) {
       // 'store' is between the current LCA and earliest possible block.
       // Label its block, and decide later on how to raise the LCA
--- a/src/hotspot/share/opto/phasetype.hpp	Wed May 23 15:21:54 2018 +0200
+++ b/src/hotspot/share/opto/phasetype.hpp	Wed Mar 21 16:03:12 2018 +0100
@@ -49,6 +49,7 @@
   PHASE_BEFORE_BEAUTIFY_LOOPS,
   PHASE_AFTER_BEAUTIFY_LOOPS,
   PHASE_BEFORE_MATCHING,
+  PHASE_MATCHING,
   PHASE_INCREMENTAL_INLINE,
   PHASE_INCREMENTAL_BOXING_INLINE,
   PHASE_END,
@@ -83,7 +84,8 @@
       case PHASE_AFTER_CLOOPS:               return "After CountedLoop";
       case PHASE_BEFORE_BEAUTIFY_LOOPS:      return "Before beautify loops";
       case PHASE_AFTER_BEAUTIFY_LOOPS:       return "After beautify loops";
-      case PHASE_BEFORE_MATCHING:            return "Before Matching";
+      case PHASE_BEFORE_MATCHING:            return "Before matching";
+      case PHASE_MATCHING:                   return "After matching";
       case PHASE_INCREMENTAL_INLINE:         return "Incremental Inline";
       case PHASE_INCREMENTAL_BOXING_INLINE:  return "Incremental Boxing Inline";
       case PHASE_END:                        return "End";
--- a/test/hotspot/jtreg/compiler/c2/Test6843752.java	Wed May 23 15:21:54 2018 +0200
+++ b/test/hotspot/jtreg/compiler/c2/Test6843752.java	Wed Mar 21 16:03:12 2018 +0100
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 6843752
+ * @bug 6843752 8192992
  * @summary missing code for an anti-dependent Phi in GCM
  *
  * @run main/othervm -Xbatch compiler.c2.Test6843752