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

[Tobias Hartmann]

Hi Roland,

On 30.10.2015 16:07, Roland Westrelin wrote:
>> 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?

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

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

http://cr.openjdk.java.net/~thartmann/8140574/webrev.04/

Thanks,
Tobias

> 
> Otherwise it looks good to me.
> 
> Roland.
>