[9] RFR(S): 8140574: C2 must re-execute checks after deoptimizing from merged uncommon traps

[Roland Westrelin]

> I see. So the only pattern we are looking for is where 'throw' is replaced with unstable_if uncommon trap.

Actually, thinking about this more, I don’t think testing for unstable_if is sufficient. We could have something like this:

if (i < 0) {
  if (some_condition) {
    // never taken so unc unstable_if
  }
}
if (i >= length) {
  // never taken so unc unstable_if
}

Then if the compiler proves some_condition is always true, we have:

if (i < 0) {
  // never taken so unc unstable_if
}
if (i >= length) {
  // never taken so unc unstable_if
}

and we go ahead with the folding but the deoptimizatin in the first branch would have us resume execution after the i < 0 test.

So I think we have to restrict when we apply that transformation even more, maybe matching the expected bytecode pattern more closely?

Roland.

> 
> Okay then.
> 
> Thanks,
> Vladimir
> 
> On 10/27/15 11:09 PM, Roland Westrelin wrote:
>>> Should we just additionally check re-execute flag?
>>> 
>>> dom_unc->jvms()->should_reexecute()
>>> 
>>> Unfortunately wee can't change reexecute flag since JVM state is already recorded according to it.
>>> 
>>> Roland, what do you think?
>> 
>> Is checking for should_reexecute() sufficient? Couldn’t we reexecute some bytecode in one branch that caused the branch to become dead but not the if itself?
>> 
>> This change was put in so we optimize the pattern:
>> 
>> if (i < 0 || i >= length) { throw… }
>> 
>> of explicit array bound checks. Restricting it to unstable_if uncommon traps, doesn’t break that scenario so I would say that fix is good enough and feels safe.
>> 
>> Roland.
>>