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
--- 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