[llvm-commits] [PATCH] Fixed ADR in ARM code generator

[Greg Fitzgerald]

Owen,

> Immediate displacements (as opposed to labels) are an _undocumented_
> extension of ARM unified syntax

Thanks for the clarification.  Diving deeper, I see the syntax is
documented as the "alternative syntax" and is explicitly discouraged
by ARM for this instruction.

"""
Note ARM recommends that where possible, software avoids using:
• the alternative syntax for the ADR, LDC, LDC2, LDR, LDRB, LDRD,
LDRH, LDRSB, LDRSH, PLD, PLI, PLDW, and VLDR
instructions
"""


> I'd much rather see the assembly parser enhanced to reject them altogether

I agree, we should make the parser reject this syntax.


> the intended design of the MC layer is that it should not be possible to
> construct an MCInst representing an unencodable instruction

I see, good rule.  Is this, or other rules I should know about,
documented somewhere?

Thanks,
Greg


On Tue, Sep 18, 2012 at 2:53 PM, Owen Anderson <resistor at mac.com> wrote:
> Greg,
>
> As I explained in another thread, the intended design of the MC layer is that it should not be possible to construct an MCInst representing an unencodable instruction.  Because of that, the design is such that immediate values of branches offsets much be pre-shifted as appropriate when constructing the MCInst itself.  Otherwise, it would be possible to construct a branch with an odd displacement value, which cannot be encoded.
>
> I don't consider the current behavior to be incorrect.  Immediate displacements (as opposed to labels) are an _undocumented_ extension of ARM unified syntax, and as such their interpretation reflects the assembler's internal model.  I'd much rather see the assembly parser enhanced to reject them altogether than rework the MC model to make them "work" in some abstract sense.
>
> --Owen
>
>
> On Sep 17, 2012, at 10:27 AM, Greg Fitzgerald <gregf at codeaurora.org> wrote:
>
>>> There was a previous thread on this topic, but that bug
>>> should have been closed as invalid, I believe.
>>
>> Thanks Gordon.  Stepan's patch would fix the problem with encoding
>> immediates, but agree with Owen that it should break ADR when the argument
>> is a label, because it would be shifted twice.  The patch I submitted fixes
>> only the immediates and doesn't affect fixups.  It also makes sure the
>> immediate is a multiple of 4 and within the range 0 to 1020.  I've updated
>> the unit tests and walked through relaxation in the debugger to verify the
>> fix.  I would have liked to see post-relaxation unit tests as well, but
>> didn't spot any.  If we have those for ADR or for other instructions, can
>> someone kindly point me to an example?
>>
>> Owen, Anton, can you please have a look at my patch?
>>
>> Thanks,
>> Greg
>>
>>
>> -----Original Message-----
>> From: Gordon Keiser [mailto:gkeiser at arxan.com]
>> Sent: Sunday, September 16, 2012 12:00 PM
>> To: Greg Fitzgerald; llvm-commits at cs.uiuc.edu
>> Subject: RE: [llvm-commits] [PATCH] Fixed ADR in ARM code generator
>>
>>
>>> From: llvm-commits-bounces at cs.uiuc.edu
>>> [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Greg Fitzgerald
>>> Sent: Friday, September 14, 2012 2:26 PM
>>> To: llvm-commits at cs.uiuc.edu
>>> Subject: [llvm-commits] [PATCH] Fixed ADR in ARM code generator
>>>
>>> My first patch to LLVM, please review.
>>>
>>> This addresses bug 13537: http://llvm.org/bugs/show_bug.cgi?id=13537
>>>
>>> Thanks,
>>> Greg
>>> --
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>>> hosted by The Linux Foundation
>>
>> There was a previous thread on this topic, but that bug should have been
>> closed as invalid, I believe.
>> getThumbAdrLabelOpValue retrieves the branch target using the fixup type
>> fixup_thumb_adr_pcrel_10, which is defined in ARMAsmBackend.cpp to return:
>> ((Value - 4) >> 2) & 0xff
>>
>> Original message on gmane
>> http://permalink.gmane.org/gmane.comp.compilers.llvm.cvs/119492
>> Someone with more extensive knowledge of the ARM backend can probably tell
>> you more, you may be fixing something somewhat different.
>>
>> Cheers,
>> Gordon Keiser
>> Software Development Engineer
>> Arxan Technologies
>> gkeiser at arxan.com  www.arxan.com
>>
>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits