[LLVMdev] Possible bug in the ARM backend?

[Roman Levenstein]

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