RFR(XS) 8026939: assert(Reachblock != NULL) failed: Reachblock must be non-NULL

[Niclas Adlertz]

Thanks Chris.

Kind Regards,
Niclas Adlertz

On 2013-10-23 02:20, Christian Thalinger wrote:
>
> On Oct 22, 2013, at 2:35 AM, Niclas Adlertz <niclas.adlertz at oracle.com> wrote:
>
>> CTW and JPRT doesn't trigger this (that's why it was pushed unnoticed).
>> However, I have tested 'java/lang/Character/CheckScript.java' with this fix and it now works.
>>
>> (java/lang/Character/CheckScript.java was a failing test in nightlies)
>
> Okay.  Looks good.
>
>>
>> Kind Regards,
>> Niclas Adlertz
>>
>> On 2013-10-22 05:27, Christian Thalinger wrote:
>>> What testing did you do?
>>>
>>> On Oct 21, 2013, at 7:08 AM, Niclas Adlertz <niclas.adlertz at oracle.com> wrote:
>>>
>>>> Hi all,
>>>>
>>>> Now that more nodes can be rematerialized due to fix JDK-8022783, a rematerialized node can have a spilled live range as input even when rematerializing input for phi nodes (in the Phi Node Splitting pass).
>>>> Prior to the JDK-8013830 and JDK-8005956 fixes, the Reachblock pointer could be wrong (hence the warning in JDK-8013830). Now, we pass in the correct block-reaches, i.e. the reaches for the 'pred' block.
>>>>
>>>> Kind Regards,
>>>> Niclas Adlertz
>>>>
>>>> JBS:    https://bugs.openjdk.java.net/browse/JDK-8026939
>>>> WEBREV: cr.openjdk.net is down so I'm pasting the diff here:
>>>>
>>>> diff -r 52575a17a36c src/share/vm/opto/reg_split.cpp
>>>> --- a/src/share/vm/opto/reg_split.cpp	Mon Oct 21 09:21:41 2013 +0400
>>>> +++ b/src/share/vm/opto/reg_split.cpp	Mon Oct 21 15:34:42 2013 +0200
>>>> @@ -365,7 +365,6 @@
>>>>        }
>>>>         if (lidx < _lrg_map.max_lrg_id() && lrgs(lidx).reg() >= LRG::SPILL_REG) {
>>>> -        assert(Reachblock != NULL, "Reachblock must be non-NULL");
>>>>          Node *rdef = Reachblock[lrg2reach[lidx]];
>>>>          if (rdef) {
>>>>            spill->set_req(i, rdef);
>>>> @@ -476,7 +475,6 @@
>>>>     uint                 bidx, pidx, slidx, insidx, inpidx, twoidx;
>>>>    uint                 non_phi = 1, spill_cnt = 0;
>>>> -  Node               **Reachblock;
>>>>    Node                *n1, *n2, *n3;
>>>>    Node_List           *defs,*phis;
>>>>    bool                *UPblock;
>>>> @@ -559,7 +557,7 @@
>>>>       b  = _cfg.get_block(bidx);
>>>>      // Reaches & UP arrays for this block
>>>> -    Reachblock = Reaches[b->_pre_order];
>>>> +    Node** Reachblock = Reaches[b->_pre_order];
>>>>      UPblock    = UP[b->_pre_order];
>>>>      // Reset counter of start of non-Phi nodes in block
>>>>      non_phi = 1;
>>>> @@ -1315,6 +1313,7 @@
>>>>        pidx = pred->_pre_order;
>>>>        // Grab reaching def
>>>>        Node *def = Reaches[pidx][slidx];
>>>> +      Node** Reachblock = Reaches[pidx];
>>>>        assert( def, "must have reaching def" );
>>>>        // If input up/down sense and reg-pressure DISagree
>>>>        if (def->rematerialize()) {
>>>> @@ -1327,8 +1326,7 @@
>>>>                 _lrg_map.find(pred->get_node(insert - 1)) >= lrgs_before_phi_split) {
>>>>            insert--;
>>>>          }
>>>> -        // since the def cannot contain any live range input, we can pass in NULL as Reachlock parameter
>>>> -        def = split_Rematerialize(def, pred, insert, maxlrg, splits, slidx, lrg2reach, NULL, false);
>>>> +        def = split_Rematerialize(def, pred, insert, maxlrg, splits, slidx, lrg2reach, Reachblock, false);
>>>>          if (!def) {
>>>>            return 0;    // Bail out
>>>>          }
>>>
>>
>