[llvm] r265317 - Revert r265309 and r265312 because they caused some errors I need to investigate.
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 22 12:57:12 PDT 2016
Thanks for the heads-up Wei.
Cheers,
Q.
> On Apr 22, 2016, at 12:38 PM, Wei Mi <wmi at google.com> wrote:
>
> Hi Quentin,
>
> First I want to thank you for so much help to review and improve the
> patches. I didn't receive bug reports for some days and I thought
> maybe I can tell you it is good to reapply the r263460, however
> yesterday Mikael Holmén sent another two error reports to me. The
> first one is easy to fix and I have tested the patch. I am still
> thinking the best way to fix the other one. Here are the reports.
>
> Thanks,
> Wei.
>
> ==================================================================================
> The first report:
> ==================================================================================
> After even more testing we've run into a new problem related to your commit
> "Recommit r265547, and r265610,r265639,r265657 on top of it, plus"
>
> In LiveRangeEdit.cpp:
>
> if (isOrigDef) {
> LiveInterval &NewLI = createEmptyIntervalFrom(Dest);
> VNInfo *VNI = NewLI.getNextValue(Idx, LIS.getVNInfoAllocator());
> NewLI.addSegment(LiveInterval::Segment(Idx, Idx.getDeadSlot(), VNI));
> pop_back();
> markDeadRemat(MI);
> const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo();
> MI->substituteRegister(Dest, NewLI.reg, 0, TRI);
> MI->getOperand(0).setIsDead(true);
> } else {
>
> What if the destination of MI is tied to one of the input operands? Then
>
> MI->substituteRegister(Dest, NewLI.reg, 0, TRI);
>
> will change both the def and the use to be the new vreg, and suddenly
> we get a use of an undefined register. That seems to happen for me in
> my out-of-tree target in some cases now.
>
> I turned on -debug printouts and I see:
>
> All defs dead: %vreg65<def,dead,tied1> =
> addh_nimm5_a32_aNhDst_formatDstN32 %vreg65<tied0>, 4
> Deleting dead def 1212r %vreg65<def,dead,tied1> =
> addh_nimm5_a32_aNhDst_formatDstN32 %vreg65<tied0>, 4
>
>
> When I stop at the "if" above we have:
>
> 176B BB#1: derived from LLVM BB %vector.body
> [...]
> 192B %vreg65<def,tied1> =
> addh_nimm5_a32_aNhDst_formatDstN32 %vreg65<tied0>, 4
> 212B %vreg60<def> = COPY %vreg65
> [...]
> 240B bkrepLabelPseudo <BB#1>, 0
> Successors according to CFG: BB#2 BB#1
> 1116B BB#2: derived from LLVM BB %bb3
> [...]
> 1212B %vreg65<def,dead,tied1> =
> addh_nimm5_a32_aNhDst_formatDstN32 %vreg65<tied0>, 4
>
> MI is:
> %vreg65<def,dead,tied1> = addh_nimm5_a32_aNhDst_formatDstN32 %vreg65<tied0>, 4
>
> And then after substituteRegister we get:
>
> %vreg67<def,dead,tied1> = addh_nimm5_a32_aNhDst_formatDstN32 %vreg67<tied0>, 4
>
> And then the verifier gets upset with me:
>
> *** Bad machine code: No live segment at use ***
> - function: f1
> - basic block: BB#2 bb3 (0x2248698) [1116B;1292B)
> - instruction: 1212B %vreg67<def,dead,tied1> =
> addh_nimm5_a32_aNhDst_formatDstN32
> - operand 1: %vreg67<tied0>
> - liverange: [1212r,1212d:0) 0 at 1212r
> - v. register: %vreg67
> - at: 1212B
>
> *** Bad machine code: Virtual register def doesn't dominate all uses. ***
> - function: f1
> - basic block: BB#2 bb3 (0x2248698) [1116B;1292B)
> - instruction: 1212B %vreg67<def,dead,tied1> =
> addh_nimm5_a32_aNhDst_formatDstN32
> LLVM ERROR: Found 2 machine code errors.
>
> So if the def operand is tied, then we get a use of the new register
> which has no dominating def and the live segment is broken.
>
> ==================================================================================
> The second report:
> ==================================================================================
> However, now we've found another problem in the same region.
>
> From the printouts I see:
>
> %vreg29 [880r,896r:0) 0 at 880r
> [...]
> 880B %vreg29<def> = COPY %a0h
> 896B %vreg10<def> = mv_ar16_ar16_lo16In32 %vreg29
> [...]
> Deleting dead def 896r %vreg81<def,dead> = mv_ar16_ar16_lo16In32 %vreg29
> [...]
> Shrink: %vreg29 [880r,896r:0) 0 at 880r
> Shrunk: %vreg29 [880r,896r:0) 0 at 880r
> [...]
>
> And then the verifier says:
>
> *** Bad machine code: Live segment doesn't end at a valid instruction ***
> - function: f3
> - basic block: BB#4 bb3 (0x42e13d8) [816B;976B)
> - liverange: [880r,896r:0) 0 at 880r
> - register: %vreg29
> - segment: [880r,896r:0)
> LLVM ERROR: Found 1 machine code errors.
>
> Notice the "Shrunk" printout above. The new range is the same as the old one!
>
> Could this be because in LiveRangeEdit::eliminateDeadDef we collect
> LiveIntervals in ToShrinkSet &ToShrink, and then someone goes over the
> set afterwards and shrinks the LIs now that the dead def is removed.
>
> However, with your changes in eliminateDeadDef the dead def is still
> there when attempting the shrinking, so the LI can't be shrunken at
> this time.
>
> Then when doing the actual removal of the dead def in postOptimization
> we currently don't do anything as far as I can see to update the LIs
> of the vregs used in the dead def instruction, and we can end up with
> invalid intervals as the verifier says.
>
> On Fri, Apr 22, 2016 at 12:01 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>> Hi Wei,
>>
>> The spill hoisting landed for good now, right?
>>
>> I basically want to know if the spiller is ready for me to reapply r263460.
>>
>> Cheers,
>> -Quentin
>>> On Apr 4, 2016, at 11:50 AM, Wei Mi <wmi at google.com> wrote:
>>>
>>> On Mon, Apr 4, 2016 at 11:45 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>>>>
>>>>> On Apr 4, 2016, at 11:43 AM, Wei Mi <wmi at google.com> wrote:
>>>>>
>>>>> My patch caused llvm bootstrap error on several targets. I am looking
>>>>> at it. Will fix the error as soon as possible.
>>>>
>>>> Do you have a link?
>>>>
>>>> Q.
>>>>
>>>
>>> Here:
>>>
>>> bootstrap errors:
>>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/1681/steps/build%20stage%202/logs/stdio
>>> http://lab.llvm.org:8011/builders/clang-3stage-ubuntu/builds/806/steps/build-stage3-clang/logs/stdio
>>>
>>> test error on aarch64:
>>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/5936/steps/test-suite/logs/test.log
>>>
>>> Wei.
>>
More information about the llvm-commits
mailing list