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

Roland Westrelin roland.westrelin at oracle.com
Wed Nov 4 09:18:58 UTC 2015

Hi Tobias,

>>> http://cr.openjdk.java.net/~thartmann/8140574/webrev.03/
>> I think you need to check that should_reexecute() is true for the JVMState of the dominating test. 
> I added the check. Should we then remove the filter for "Reason_unstable_if" and handle all types of uncommon traps as long as they have the re-execute flag set?

Yes, I think you can remove that check.

>> I don’t know if that part is good or not:
>> 2911   JsrSet* jsr_null = new ciTypeFlow::JsrSet(NULL);
>> 2914   Block*     block = get_block_for(index, jsr_null, ciTypeFlow::no_create);
>> 2915   Block* dom_block = get_block_for(dom_index, jsr_null, ciTypeFlow::no_create);
>> (The use of JsrSet null that I don’t understand)
> I think the JsrSet is used during data flow analysis to record all the active jump to subroutine (jsr) instructions with destination and return address.
> // During abstract interpretation, JsrSets are used to determine
> // whether two paths which reach a given block are unique, and
> // should be cloned apart, or are compatible, and should merge
> // together.
> I assumed that for our purpose of checking domination this does not matter and therefore used an empty JsrSet as it's done in ciTypeFlow::flow_types() and ciTypeFlow::get_start_state(). JsrSet::is_compatible_with(JsrSet* other) returns true if 'other' is empty.
> Do you think we have to check domination for the blocks corresponding to all possible JsrSets at this bci?

I’m still unsure this is correct in the presence of jsrs. What if we reach a block from multiple jsrs? Wouldn’t there be cases where they are in different jsr sets? jsrs are uncommon enough AFAIU that I would bail out of this optimization if the method has any (ciMethod::has_jsrs()).

>> Also, the same bci can be in multiple blocks because ciTypeFlow clones some block (ciTypeFlow::clone_loop_head()) and I wonder if that can be a problem or not.
> We get the block via ciTypeFlow::get_block_for() which checks block->is_backedge_copy() and therefore should only return the "original" block and no clones (clones are marked by get_block_for() if invoked with 'create_backedge_copy'). So I don't think this is a problem.

I think that’s good then.

ciTypeFlow::clone_loop_head() updates successor blocks so you need to update predecessors too. Also, what about exception edges for predecessors? I think you need to take them into account too.

I also think you need to check that the jvms stacks of method/bci are identical for both uncommon traps except for the top bci to make sure we got to the method from the same call stack.


> http://cr.openjdk.java.net/~thartmann/8140574/webrev.04/
> Thanks,
> Tobias
>> Otherwise it looks good to me.
>> Roland.

More information about the hotspot-compiler-dev mailing list