[PATCH] Fix implementation for Thumb ADR instruction

Tim Northover t.p.northover at gmail.com
Tue Jul 2 04:56:17 PDT 2013


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.



More information about the llvm-commits mailing list