[LLVMdev] Possible bug in the ARM backend?

[Evan Cheng]

This looks like a bar in ARMInstrInfo.td:

BX_RET should be marked with Uses = [LR] since it uses LR. However,  
this won't work if there is a call BL before the BX_RET. BL is marked  
as if it implicitly define LR. So we'll end up with this (hello world  
example):

Live Ins: %LR %R7
         %SP<def> = SUBri %SP<kill>, 8, 14, %reg0, %reg0
         STR %LR<kill>, %SP, %reg0, 4, 14, %reg0
         STR %R7<kill>, %SP, %reg0, 0, 14, %reg0
         %R7<def> = MOVr %SP, 14, %reg0, %reg0
         %R0<def> = LDR <cp#0>, %reg0, 0, 14, %reg0, Mem:LD(4,4)  
[<unknown> + 0]
         BL <ga:puts>, %R0<kill>, %R0<imp-def,dead>, %R1<imp- 
def,dead>, %R2<imp-def,dead>, %R3<imp-def,dead>, %R12<imp-def,dead>,  
%LR<imp-def>, %D0<imp-def,dead>, %D1<imp-def,de\
ad>, %D2<imp-def,dead>, %D3<imp-def,dead>, %D4<imp-def,dead>, %D5<imp- 
def,dead>, %D6<imp-def,dead>, %D7<imp-def,dead>, %CPSR<imp-def,dead>
         %R0<def> = MOVi 0, 14, %reg0, %reg0
         %R7<def> = LDR %SP, %reg0, 0, 14, %reg0
         %LR<def> = LDR %SP, %reg0, 4, 14, %reg0
	%SP<def> = ADDri %SP<kill>, 8, 14, %reg0, %reg0
         BX_RET 14, %reg0, %R0<imp-use,kill>, %LR<imp-use,kill>

The LR defined by BL is not killed before the PEI inserted LR restore.  
The register scavenger doesn't like this.

The issue is while BL does modifies LR, it doesn't actually defines LR  
so later instructions can use it. I'll think about how to fix this.   
It's not obvious to me at this point.

Evan

On Jan 7, 2009, at 2:24 PM, Roman Levenstein wrote:

> Hi Evan,
>
> Thanks for your feedback!
>
> 2009/1/7 Evan Cheng <evan.cheng at apple.com>:
>>
>> On Jan 7, 2009, at 2:48 AM, Roman Levenstein wrote:
>>
>>
>> As you can see, PrologEpilogInserter has inserted at the beginning
>> of the function some code for manipulation of the frame pointer and
>> this inserted code uses the LR register.
>> As far as I understand,  ARMRegisterInfo.td should exclude the LR
>> register from the set of allocatable registers for functions that
>> require frame pointer manipulation.
>> But currently it is not the case, or?
>>
>> No, LR is not the frame pointer. It's the link register (caller  
>> address). It
>> should be available as a general purpose register.
>
> OK.
>
>> The bug is elsewhere. It  has to do with kill / dead markers.
>>       %LR<def> = LDR <fi#0>, %reg0, 0, 14, %reg0
>>       %SP<def> = ADDri %SP<kill>, 4, 14, %reg0, %reg0
>>       BX_RET 14, %reg0
>> LR is restored here but it's not killed before the end of the block  
>> is
>> reached.
>
> Hmm. I have no idea about what ARM backend does. My register allocator
> just assigns the registers as I explained in my original mail.
> Then it lets VirtRegMap.cpp do its job, i.e. it lets it rewrite the
> code and replace virtual registers by the assigned physical registers.
> You can see the result in the step (3) of my original mail. In my
> opinion, it still looks correct. May be this rewriting process does
> something wrong?
>
> Then PrologEpilogInserter and some other standard post RA passes are
> invoked for the ARM backend. But I have not changed anything there, so
> I have no idea what happens.
>
> And, BTW, the instructions you mentioned above are after the
> instruction triggering the assertion, which is:
> STR %LR<kill>, %R0<kill>, %reg0, 0, 14, %reg0, Mem:ST(4,4)  
> [0x8fc2d68 + 0]
>
>> Should BX_RET use it?
>
> I don't know the semantics of BX_RET on the ARM platform. May be it
> uses BX_RET somehow.
>
> BTW, an idea: May be it is easy to trigger exactly the same behaviour
> with the linear scan if one does the following:
>  - comment out dependency on the coalescer, so that it is not invoked
>  - change the allocation order of the GPR register class for ARM, so
> that it starts with the LR register.
> ,
> Any ideas how to proceed with the current situation?
>
> -Roman
>
>> I hope that I provided enough information to explain my problem. I
>> also provided my initial analysis, but may be I'm wrong.
>>
>> Can someone more knowledgeable in ARM backend and LLVM's register
>> allocation framework have a look at it?
>> If it is a bug in the ARM backend, could it be fixed?
>>
>> Thanks,
>>  Roman
>> <bugpoint-reduced-simplified.bc>
>>