[LLVMdev] Possible bug in the ARM backend?

[Roman Levenstein]

Hi Evan,

OK, I got it. Your advice about LiveIn regs saved my day! Now ARM
failures are in the past.

Thanks a lot!
-Roman


2009/1/13 Roman Levenstein <romix.llvm at googlemail.com>:
> Hi again,
>
> 2009/1/13 Evan Cheng <evan.cheng at apple.com>:
>>
>>
>> On Jan 13, 2009, at 12:27 AM, Roman Levenstein <romix.llvm at googlemail.com>
>> wrote:
>>
>>> 2009/1/13 Evan Cheng <echeng at apple.com>:
>>>>
>>>> On Jan 7, 2009, at 2:48 AM, Roman Levenstein wrote:
>>>>
>>>>> bb368: 0x8fdad00, LLVM BB @0x8fc2c98, ID#1:
>>>>>  Predecessors according to CFG: 0x8fdac90 (#0)
>>>>>     %R0<def> = MOVi 0, 14, %reg0, %reg0
>>>>> ***     STR %LR<kill>, %R0<kill>, %reg0, 0, 14, %reg0, Mem:ST(4,4)
>>>>> [0x8fc2d68 + 0]
>>>>>     %LR<def> = LDR <fi#0>, %reg0, 0, 14, %reg0
>>>>>     %SP<def> = ADDri %SP<kill>, 4, 14, %reg0, %reg0
>>>>>     BX_RET 14, %reg0
>>>>
>>>> Ok, ignore my earlier email about BX_RET. The issue is LR should be added
>>>> to
>>>> livein of BB #1.
>>>
>>> Who should do it?
>>> Do you mean that ARM backend/LiveIntervalsAnalysis/LiveVariables
>>> should do it or do you mean that my regalloc should do it?
>>
>> Register allocator should update mbb Livein info.
>
> OK.
>
>>>
>>>
>>>> **** Post Machine Instrs ****
>>>> # Machine code for Insert():
>>>> Live Ins: R0 in VR#1025  R1 in VR#1026
>>>>
>>>> entry: 0x8fdac90, LLVM BB @0x8fc2c48, ID#0:
>>>> Live Ins: %R0 %R1
>>>>     %LR<def> = MOVr %R0, 14, %reg0, %reg0
>>>>     CMPri %R0<kill>, 0, 14, %reg0, %CPSR<imp-def>
>>>>     Bcc mbb<UnifiedReturnBlock,0x8fdad70>, 10, %CPSR<kill>
>>>>  Successors according to CFG: 0x8fdad00 (#1) 0x8fdad70 (#2)
>>>>
>>>> bb368: 0x8fdad00, LLVM BB @0x8fc2c98, ID#1:
>>>>  Predecessors according to CFG: 0x8fdac90 (#0)
>>>>     %R0<def> = MOVi 0, 14, %reg0, %reg0
>>>>     STR %LR<kill>, %R0<kill>, %reg0, 0, 14, %reg0, Mem:ST(4,4)
>>>> [0x8fc2d68 + 0]
>>>>     BX_RET 14, %reg0
>>>>
>>>> Here the STR is using LR, but there isn't a def earlier.
>>>
>>> May be I overlook something, but doesn't
>>> %LR<def> = MOVr %R0, 14, %reg0, %reg0
>>> in MBB#0 define the LR? It should be enough, or?
>>
>> Every machine basic block must list physical register livein's.
>
> One question, just to be sure I understand you correctly. You mean
> that after the RegAlloc has assigned physical registers to
> LiveIntervals and before it calls the VRM to rewrite the function, it
> should explicitly add lives-ins for each MBB, just like LinearScan
> does it at the end of the RALinScan::linearScan() function???
>
> E.g. like this:
>
>  // Add live-ins to every BB except for entry. Also perform trivial coalescing.
>  MachineFunction::iterator EntryMBB = mf_->begin();
>  SmallVector<MachineBasicBlock*, 8> LiveInMBBs;
>  for (LiveIntervals::iterator i = li_->begin(), e = li_->end(); i != e; ++i) {
>    LiveInterval &cur = *i->second;
>    unsigned Reg = 0;
>    bool isPhys = TargetRegisterInfo::isPhysicalRegister(cur.reg);
>    if (isPhys)
>      Reg = cur.reg;
>    else if (vrm_->isAssignedReg(cur.reg))
>      Reg = attemptTrivialCoalescing(cur, vrm_->getPhys(cur.reg));
>    if (!Reg)
>      continue;
>    // Ignore splited live intervals.
>    if (!isPhys && vrm_->getPreSplitReg(cur.reg))
>      continue;
>    for (LiveInterval::Ranges::const_iterator I = cur.begin(), E = cur.end();
>         I != E; ++I) {
>      const LiveRange &LR = *I;
>      if (li_->findLiveInMBBs(LR.start, LR.end, LiveInMBBs)) {
>        for (unsigned i = 0, e = LiveInMBBs.size(); i != e; ++i)
>          if (LiveInMBBs[i] != EntryMBB)
>            LiveInMBBs[i]->addLiveIn(Reg);
>        LiveInMBBs.clear();
>      }
>    }
>  }
>
> If it is the case, it is OK. It was not clear for me that one has to
> do it in the regalloc. My assumption was that VRM would do it.
>
> Thanks a lot,
>  Roman
>