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

[Niclas Adlertz]

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.