Wed, 15 Feb 2012 09:43:16 +0100
7126041: jdk7u4 b05 and b06 crash with RubyMine 3.2.4, works well with b04
Summary: Goto that replaces a If mistaken to be a back branch and triggers erroneous OSR compilation.
Reviewed-by: never, iveresov
src/share/vm/c1/c1_Canonicalizer.cpp | file | annotate | diff | comparison | revisions | |
src/share/vm/c1/c1_GraphBuilder.cpp | file | annotate | diff | comparison | revisions |
1.1 --- a/src/share/vm/c1/c1_Canonicalizer.cpp Tue Feb 14 15:43:56 2012 -0800 1.2 +++ b/src/share/vm/c1/c1_Canonicalizer.cpp Wed Feb 15 09:43:16 2012 +0100 1.3 @@ -594,6 +594,13 @@ 1.4 return false; 1.5 } 1.6 1.7 +static bool is_safepoint(BlockEnd* x, BlockBegin* sux) { 1.8 + // An Instruction with multiple successors, x, is replaced by a Goto 1.9 + // to a single successor, sux. Is a safepoint check needed = was the 1.10 + // instruction being replaced a safepoint and the single remaining 1.11 + // successor a back branch? 1.12 + return x->is_safepoint() && (sux->bci() < x->state_before()->bci()); 1.13 +} 1.14 1.15 void Canonicalizer::do_If(If* x) { 1.16 // move const to right 1.17 @@ -614,7 +621,7 @@ 1.18 case If::geq: sux = x->sux_for(true); break; 1.19 } 1.20 // If is a safepoint then the debug information should come from the state_before of the If. 1.21 - set_canonical(new Goto(sux, x->state_before(), x->is_safepoint())); 1.22 + set_canonical(new Goto(sux, x->state_before(), is_safepoint(x, sux))); 1.23 return; 1.24 } 1.25 1.26 @@ -626,7 +633,7 @@ 1.27 x->sux_for(false)); 1.28 if (sux != NULL) { 1.29 // If is a safepoint then the debug information should come from the state_before of the If. 1.30 - set_canonical(new Goto(sux, x->state_before(), x->is_safepoint())); 1.31 + set_canonical(new Goto(sux, x->state_before(), is_safepoint(x, sux))); 1.32 } 1.33 } 1.34 } else if (rt->as_IntConstant() != NULL) { 1.35 @@ -694,10 +701,12 @@ 1.36 } 1.37 } else if (rt == objectNull && (l->as_NewInstance() || l->as_NewArray())) { 1.38 if (x->cond() == Instruction::eql) { 1.39 - set_canonical(new Goto(x->fsux(), x->state_before(), x->is_safepoint())); 1.40 + BlockBegin* sux = x->fsux(); 1.41 + set_canonical(new Goto(sux, x->state_before(), is_safepoint(x, sux))); 1.42 } else { 1.43 assert(x->cond() == Instruction::neq, "only other valid case"); 1.44 - set_canonical(new Goto(x->tsux(), x->state_before(), x->is_safepoint())); 1.45 + BlockBegin* sux = x->tsux(); 1.46 + set_canonical(new Goto(sux, x->state_before(), is_safepoint(x, sux))); 1.47 } 1.48 } 1.49 } 1.50 @@ -710,7 +719,7 @@ 1.51 if (v >= x->lo_key() && v <= x->hi_key()) { 1.52 sux = x->sux_at(v - x->lo_key()); 1.53 } 1.54 - set_canonical(new Goto(sux, x->state_before(), x->is_safepoint())); 1.55 + set_canonical(new Goto(sux, x->state_before(), is_safepoint(x, sux))); 1.56 } else if (x->number_of_sux() == 1) { 1.57 // NOTE: Code permanently disabled for now since the switch statement's 1.58 // tag expression may produce side-effects in which case it must 1.59 @@ -741,7 +750,7 @@ 1.60 sux = x->sux_at(i); 1.61 } 1.62 } 1.63 - set_canonical(new Goto(sux, x->state_before(), x->is_safepoint())); 1.64 + set_canonical(new Goto(sux, x->state_before(), is_safepoint(x, sux))); 1.65 } else if (x->number_of_sux() == 1) { 1.66 // NOTE: Code permanently disabled for now since the switch statement's 1.67 // tag expression may produce side-effects in which case it must
2.1 --- a/src/share/vm/c1/c1_GraphBuilder.cpp Tue Feb 14 15:43:56 2012 -0800 2.2 +++ b/src/share/vm/c1/c1_GraphBuilder.cpp Wed Feb 15 09:43:16 2012 +0100 2.3 @@ -1181,6 +1181,11 @@ 2.4 bool is_bb = tsux->bci() < stream()->cur_bci() || fsux->bci() < stream()->cur_bci(); 2.5 Instruction *i = append(new If(x, cond, false, y, tsux, fsux, is_bb ? state_before : NULL, is_bb)); 2.6 2.7 + assert(i->as_Goto() == NULL || 2.8 + (i->as_Goto()->sux_at(0) == tsux && i->as_Goto()->is_safepoint() == tsux->bci() < stream()->cur_bci()) || 2.9 + (i->as_Goto()->sux_at(0) == fsux && i->as_Goto()->is_safepoint() == fsux->bci() < stream()->cur_bci()), 2.10 + "safepoint state of Goto returned by canonicalizer incorrect"); 2.11 + 2.12 if (is_profiling()) { 2.13 If* if_node = i->as_If(); 2.14 if (if_node != NULL) { 2.15 @@ -1303,7 +1308,16 @@ 2.16 // add default successor 2.17 sux->at_put(i, block_at(bci() + sw.default_offset())); 2.18 ValueStack* state_before = has_bb ? copy_state_before() : NULL; 2.19 - append(new TableSwitch(ipop(), sux, sw.low_key(), state_before, has_bb)); 2.20 + Instruction* res = append(new TableSwitch(ipop(), sux, sw.low_key(), state_before, has_bb)); 2.21 +#ifdef ASSERT 2.22 + if (res->as_Goto()) { 2.23 + for (i = 0; i < l; i++) { 2.24 + if (sux->at(i) == res->as_Goto()->sux_at(0)) { 2.25 + assert(res->as_Goto()->is_safepoint() == sw.dest_offset_at(i) < 0, "safepoint state of Goto returned by canonicalizer incorrect"); 2.26 + } 2.27 + } 2.28 + } 2.29 +#endif 2.30 } 2.31 } 2.32 2.33 @@ -1338,7 +1352,16 @@ 2.34 // add default successor 2.35 sux->at_put(i, block_at(bci() + sw.default_offset())); 2.36 ValueStack* state_before = has_bb ? copy_state_before() : NULL; 2.37 - append(new LookupSwitch(ipop(), sux, keys, state_before, has_bb)); 2.38 + Instruction* res = append(new LookupSwitch(ipop(), sux, keys, state_before, has_bb)); 2.39 +#ifdef ASSERT 2.40 + if (res->as_Goto()) { 2.41 + for (i = 0; i < l; i++) { 2.42 + if (sux->at(i) == res->as_Goto()->sux_at(0)) { 2.43 + assert(res->as_Goto()->is_safepoint() == sw.pair_at(i).offset() < 0, "safepoint state of Goto returned by canonicalizer incorrect"); 2.44 + } 2.45 + } 2.46 + } 2.47 +#endif 2.48 } 2.49 } 2.50