[llvm-commits] FIX for PR13537: incorrect ADR encoding

[Stepan Dyatkovskiy]

Hi Owen,

I tested ADR behaviour on native ARM and reread ARM reference again.

gcc emits error while compiling expressions like
adr r0, #8
Error: symbol *ABS*0x0 is in a different section

And for example for imm=10 it emits
Error: symbol *ABS*0x2 is in a different section

I also couldn't found "ADR <Rd>,#const" encoding in manual. The nearest 
is T1 ADR encoding: ADR<c> <Rd>,<label>. But as described a little bit 
below: "<label> -- The label of an instruction or literal data item 
whose address is to be loaded into <Rd>." So it seems, that we can't use 
#some-const as operand for this instruction, and it calls into question 
correctness of "test/MC/ARM/basic-thumb-instructions.s".

-Stepan.

Owen Anderson wrote:
> Stepan,
>
> This patch is not correct.  The value that is encoded into the instruction already has the two low bits shifted out.  Take a look at the fixup_thumb_adr_pcrel_10 case in adjustFixupValue() in ARMAsmBackend.cpp to see this in action.  If your patch is applied, the right shift will effectively be applied twice, resulting in the four low bits being discarded.
>
> Note that that does lead to the somewhat confusing behavior of the "adr reg, #imm" requiring the imm operand to be pre-encoded.  However, this is already as extension to the official ARM syntax, so we're not beholden to have it work any particular way.
>
> --Owen
>
> On Aug 17, 2012, at 2:26 AM, Stepan Dyatkovskiy <stpworld at narod.ru> wrote:
>
>> Hi all. Please find the fix for PR13537 in attachment:
>> llvm-mc incorrectly assembles ADR instructions for ARM
>> T1 encoding implemented incorrectly for ADR. We should store high 8 bits of immediate, that is 10 bit width value. Currently imm interpreted as 8 bit value and low 8 bits are encoded instead.
>>
>> -Stepan.
>> <pr13537.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>