7126041: jdk7u4 b05 and b06 crash with RubyMine 3.2.4, works well with b04

Wed, 15 Feb 2012 09:43:16 +0100

author
roland
date
Wed, 15 Feb 2012 09:43:16 +0100
changeset 3570
80107dc493db
parent 3569
8f4eb44b3b76
child 3571
09d00c18e323

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  

mercurial