RFR(XS) 8022783: Nashorn test fails with: assert(!def_outside->member(r))

[Christian Thalinger]

Looks good.

On Oct 17, 2013, at 3:11 AM, Niclas Adlertz <niclas.adlertz at oracle.com> wrote:

> Thanks Vladimir.
> 
> http://cr.openjdk.java.net/~adlertz/JDK-8022783/webrev01/
> 
> Kind Regards,
> Niclas Adlertz
> 
> On 2013-10-16 18:22, Vladimir Kozlov wrote:
>> Niclas,
>> 
>> You need to change/add comments in this code. Otherwise it is good.
>> 
>> Thanks,
>> Vladimir
>> 
>> On 10/16/13 7:32 AM, Niclas Adlertz wrote:
>>> Hi all,
>>> 
>>> Despite the fact that I have not been able to reproduce JDK-8022783, it generates the same error as many other bugs;
>>> JDK-7033056, JDK-8016269, JDK-8011770, JDK-8005956, JDK-8016157 and JDK-7196566.
>>> They are all related to re-materialization. One major concern when re-materializing is that each input node to the
>>> re-materialized node might need to have its live range 'L' stretched.
>>> If 'L' is defined at multiple places (due to coalescing), we might stretch one definition of 'L' beyond another definition.
>>> 
>>> Currently we handle this by doing a private copy of the input in the same block as the node that is being
>>> re-materialized. This happens at the beginning of PhaseChaitin::Split_Rematerialize:
>>> 
>>>   // The input live ranges will be stretched to the site of the new
>>>   // instruction.  They might be stretched past a def and will thus
>>>   // have the old and new values of the same live range alive at the
>>>   // same time - a definite no-no.  Split out private copies of
>>>   // the inputs.
>>>   if( def->req() > 1 ) {
>>>     for( uint i = 1; i < def->req(); i++ ) {
>>>       Node *in = def->in(i);
>>>       // Check for single-def (LRG cannot redefined)
>>>       uint lidx = _lrg_map.live_range_id(in);
>>>       if (lidx >= _lrg_map.max_lrg_id()) {
>>>         continue; // Value is a recent spill-copy
>>>       }
>>>       if (lrgs(lidx).is_singledef()) {
>>>         continue;
>>>       }
>>> 
>>>       Block *b_def = _cfg.get_block_for_node(def);
>>>       int idx_def = b_def->find_node(def);
>>>       Node *in_spill = get_spillcopy_wide( in, def, i );
>>>       if( !in_spill ) return 0; // Bailed out
>>>       insert_proj(b_def,idx_def,in_spill,maxlrg++);
>>>       if( b_def == b )
>>>         insidx++;
>>>       def->set_req(i,in_spill);
>>>     }
>>>   }
>>> 
>>> We do not need to do this for live ranges with only a single definition.
>>> We also do not do this for recently created spill copies. Recent spill copies refers to spill copies created in the
>>> current split pass, that have not yet been assigned live ranges.
>>> 
>>> The problem is that spill copies may introduce phi nodes due to different reaching definitions from different control
>>> paths. And phi nodes introduces coalescing; in the end of a split pass we coalesce phi inputs with the phi itself (for
>>> all phi nodes created in the split pass).
>>> So what happens if a phi input is a recent spill copy? The recent spill copy get coalesced with the phi (and the live
>>> range of the the spill copy and the phi will be multi defined).
>>> And what happens if this recent spill copy is input to a re-materialized node? Then we introduce the risk of stretching
>>> the live range beyond another def. (*)
>>> 
>>> This is exactly what happens in prior bugs JDK-8005956 and JDK-8016157. When I solved them, I did not know well enough
>>> what the problem was, so I disabled re-materialization for some nodes. Now I know better and instead I can add the same
>>> support for recent spill copies, i.e. I create private copies for them as well. At the same time, I can remove the old
>>> temporary solutions.
>>> 
>>> I have verified that JDK-8005956 now also works with the new solution. Unfortunately I have not been able to reproduce
>>> JDK-8016157, but I have been able to solve a new bug, JDK-7196566, using this new solution.
>>> 
>>> I have also verified that this fix does not give any performance regressions.
>>> http://sthaurora-ds.se.oracle.com/performance/rest/submit/runStatus?name=REG_SPLIT
>>> http://sthaurora-ds.se.oracle.com/performance/rest/perf/report?name=REG_SPLIT
>>> 
>>> WEBREV: http://cr.openjdk.java.net/~adlertz/JDK-8022783/webrev00/
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8022783
>>> 
>>> Kind Regards,
>>> Niclas Adlertz
>>> 
>>> (*) It is tricky because this can not happen by just coalescing a recent spill copy (as phi input) with its phi node
>>> 'P'. It happens because the input for 'P' is also a phi, which is THEN coalesced together with the recent spill copy.
>>> Simply put, we need more than one recursion of coalescing in the split pass for this to happen.
>