[PATCH] Fix implementation for Thumb ADR instruction

[Mihail Popa]

Tim,

even if it only applies to the branches, there will be plenty of
occurrences:

1. branch conditional T1 encoding - signed, 8 bit, shift 1
2. branch unconditional T2 encoding - signed, 11 bit, shift 1
3. branch conditional T3 encoding - signed 24 bit, shift 1
4. branch linked conditional T2 encoding - signed 24 bit, shift 2

For load and store literal, with sign-magnitude format:

1. T1 encoding: unsigned, 8 bit, shift 2
2. T2 encoding: signed 12 bit, no shift

These cover only functional issues, where an encoding that is too short is
chosen
over the one of correct length.

There are probably more examples if we add constraints for the purpose of
diagnostics
and error checking.

I do agree it'd be probably better to split the template in unsigned, 2's
complement signed
and magnitude signed.

Mihai



On Tue, Jul 2, 2013 at 12:56 PM, Tim Northover <t.p.northover at gmail.com>wrote:

> Hi Mihail,
>
> Thanks for working on this, it's looking better now.
>
> > Please see attached the revised patch. I have made the template function
> and
> > the new operand classes clearly refer to PC-relative addresses
>
> So how widely are you planning to apply this method? I don't think it
> will account for the other forms of ADR for example, which have the
> same quirk as LDR that Min = -Max. It seems it could work for:
>   + Branches
>   + T1 encoding of ADR
>   + ADD, SUB (even if not PC-rel, assuming we're not going to support
> "ADD r0, r0, #-4" as an alias).
>   + MOVW/MOVT
>
> And it would fail for:
>   + Other encodings of ADR (as LLVM implements them, with +/- in same
> MCInst)
>   + Loads & stores.
>
> I don't think the "PCOffset" is a particularly better description than
> "MemoryOffset" for those instructions, given that of the three kinds
> of PC-offsetting instructions (ADR, LDR (lit), B) it works for 1.5 of
> them.
>
> Perhaps it's too general, and should split into an unsigned (added
> now) and two signed variants (added as and when they're needed), not
> controlled by templates?
>
> > and changed the shifting policy.
>
> Thanks.
>
> Some more boundary tests would probably be good (e.g. that "ADR r0,
> #1020" and "ADR r0, #-4" really are the edges of what's representable
> by the T1 encoding)?
>
> Regards.
>
> Tim.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130702/c4df803b/attachment.html>