Another problem with "Recommit r265547, and r265610,r265639,r265657"

Tue Apr 26 11:19:23 PDT 2016

Hi Wei and Mikael,

> On Apr 26, 2016, at 11:16 AM, Wei Mi <wmi at google.com> wrote:
> 
> On Tue, Apr 26, 2016 at 11:01 AM, Wei Mi <wmi at google.com <mailto:wmi at google.com>> wrote:
>> +Quentin and llvm-commits.
>> 
>> On Tue, Apr 26, 2016 at 1:05 AM, Mikael Holmén
>> <mikael.holmen at ericsson.com> wrote:
>>> Hi Wei,
>>> 
>>> On 04/26/2016 02:24 AM, Wei Mi wrote:
>>>> 
>>>> Hi Mikael, after some discussions with Quentin
>>>> 
>>>> (http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160425/350786.html),
>>>> I think it is necessary for me to look at your testcases more closely,
>>>> because in theory the errors shouldn't happen. I must be missing
>>>> something.
>>>> 
>>>> Is it possible to send out preprocessed files and the command lines to
>>>> reproduce the problems you saw?
>>> 
>>> 
>>> Well, this happens only when compiling for our out-of-tree target, so
>>> unfortunately we can't do that. We need to have our hardware's set of
>>> registers and intructions etc for the register allocator to behave exactly
>>> the way it does for us. Maybe it's possible to reproduce this on some other
>>> target, but from experience it's quite hard to do that with register
>>> allocator problems. :/
>>> 
>>> So I've compiled a bugpoint-reduced version of the code again now, with and
>>> without your latest patch "Keep dead inst copy from being deleted only when
>>> the inst is rematerializable", with debug printouts on.
>>> 
>>> I added one printout in LiveRangeEdit::checkRematerializable
>>> 
>>> +  DEBUG(dbgs() << "LiveRangeEdit::checkRematerializable: Checking
>>> isTriviallyReMaterializable on: " << *DefMI);
>>>   if (!TII.isTriviallyReMaterializable(DefMI, aa)) {
>>> +     DEBUG(dbgs() << "false\n");
>>>     return false;
>>>   }
>>> +   DEBUG(dbgs() << "true\n");
>>> 
>>> and one in LiveRangeEdit::eliminateDeadDef just before your change
>>> 
>>> +    DEBUG(dbgs() << "LiveRangeEdit::eliminateDeadDef: Checking
>>> isTriviallyReMaterializable on: " << *MI);
>>> +    if (TII.isTriviallyReMaterializable(MI, AA))
>>> +      DEBUG(dbgs() << "true\n");
>>> +    else
>>> +      DEBUG(dbgs() << "false\n");
>>> +
>>>    if (isOrigDef && DeadRemats && TII.isTriviallyReMaterializable(MI, AA))
>>> {
>>>       LiveInterval &NewLI = createEmptyIntervalFrom(Dest);
>>>       VNInfo *VNI = NewLI.getNextValue(Idx, LIS.getVNInfoAllocator());
>>> 
>>> I attached the logs from the two runs.
>>> 
>>> Note that with your patch the code compiles succesfully now, so it doesn't
>>> seem to be totally rubbish :)
>>> 
>>> Of course the logs now have some stuff from our backend, some instructions
>>> and registers you don't know, but maybe you can figure out what's going on
>>> anyway. If I can add some more printouts that you want to be able to
>>> understand better what's happening, please don't hesitate to tell me and
>>> I'll add and rerun.
>>> 
>>> Thanks for looking into this,
>>> Mikael
>>> 
>> 
>> Hi Mikael,
>> 
>> Thank you very much for providing a crash log. It is very helpful. I
>> look at foo.crash.log and find a speciality in foo.red.ll:
>> The def to vreg29 at 528r is in an infinite loop. Although the def
>> actually has no use, it is not marked as dead and these segments:
>> [336B,448B:0)[528r,608B:1)[608B,720B:2) exists only because the dead
>> def at 528r.
>> 
>> Before greedy regalloc starts:
>> %vreg29 [48r,336B:3)[336B,448B:0)[528r,608B:1)[608B,720B:2)[720B,976B:3)
>> 0 at 336B-phi 1 at 528r 2 at 608B-phi 3 at 48r
>> 
>> next step:
>> %vreg29 is splitted into %vreg31 and %vreg32.
>> 
>> next step:
>> 752B:2 libcall_CRT_ll_div_r %vreg19, %vreg19, %vreg32, %vreg1,
>> %a0_32<imp-def>, %a1_32<imp-def>, %CCReg<imp-def,dead>, ...;
>> aN32_0_7:%vreg19,%vreg32,%vreg1
>> is rematerialized, and %vreg32 at 312r becomes dead.
>> 
>> next step:
>> When it deletes 312r %vreg32<def,dead> = COPY %vreg31;
>> aN32_0_7:%vreg32,%vreg31, it shrink the live range of vreg31.
>> ---------- trace extracted from foo.crash.log -----------
>> Shrink: %vreg31 [48r,312r:0)[336B,448B:1)[528r,608B:2)[608B,720B:3)
>> 0 at 48r 1 at 336B-phi 2 at 528r 3 at 608B-phi
>> live-in at 176B
>> Dead PHI at 336B may separate interval
>> All defs dead: 528r %vreg31<def,dead> = mv_ar16_ar16_lo16In32 %vreg13,
>> pred:0, pred:%noreg, pred:0, %ac0<imp-use>, %ac1<imp-use>;
>> aN32_0_7:%vreg31 aNh_0_7:%vreg13
>> Dead PHI at 608B may separate interval
>> Shrunk: %vreg31 [48r,128B:0)[176B,192r:0)[528r,528d:2)  0 at 48r 1 at x 2 at 528r 3 at x
>>  Split 2 components: %vreg31 [48r,128B:0)[176B,192r:0)[528r,528d:1)
>> 0 at 48r 1 at 528r
>> Deleting dead def 528r %vreg34<def,dead> = mv_ar16_ar16_lo16In32
>> %vreg13, pred:0, pred:%noreg, pred:0, %ac0<imp-use>, %ac1<imp-use>;
>> aN32_0_7:%vreg34
>> ---------------------------------------------------------------------
>> Note that %vreg34 is generated from %vreg31. When 312r is deleted, it
>> shrinks the live range and finds 528r is dead at the same time. !!!
>> But the def of 528r has no relationship with the use of %vreg31 at
>> 312r !!!
>> 
>> 528r %vreg34<def,dead> = mv_ar16_ar16_lo16In32 %vreg13  is not
>> triviallyRematerializable, but because it is an OriginalDef, it can
>> satisfy the condition to go into DeadRemat. This is how we get a case
>> that an instruction in DeadRemat is not triviallyRematerializable.
>> 
>> Thanks,
>> Wei.
> 
> Hi Quentin,
> 
> From the analysis of crash report given by Mikael, the error happens
> in a very rare case -- dead live interval and dead VNI are created
> before regalloc, and then stay there until live range shrinking is
> called because of remat for other unrelated uses. In such case,
> non-triviallyRematerializable instructions can slip into DeadRemat
> set.
> 
> I think adding triviallyRematerializable check can guard the safety
> for this rare case for sure, but I don't know if we have a better way
> to remove the weird case earlier -- dead live intervals and dead VNI
> should be removed before regalloc.

I agree, those must be removed before regalloc.
Mikaeal, when are they introduced and why do they stick until regalloc?

Thanks,
-Quentin

> 
> Thanks,
> Wei.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160426/9e44eafc/attachment.html>