[PATCH] Fix implementation for Thumb ADR instruction

[Jim Grosbach]

On Jun 26, 2013, at 9:37 AM, Tim Northover <t.p.northover at gmail.com> wrote:

> Hi Mihail,
> 
> +  template<unsigned width, unsigned scale, bool positive>
> +  bool isMemoryOffset() const {
> 
> This seems rather general given that it's just used once. I'd suggest
> either stripping it back or doing a separate refactoring to
> consolidate things.
> 
> Also, unless I'm mistaken its constraints are appropriate for 2s
> complement arithmetic while ARM usually uses sign/magnitude for its
> offsets. At the very least this assumption should be commented.
> 
> +  return MO.getImm() >> 2;
> 
> I think the preferred approach is to use an already-shifted MCOperand
> so that invalid instructions can't exist by design. That removes the
> need for the decoder, but makes your AsmParser render methods (which
> currently seem pretty much identical to addImm) useful again. CodeGen
> may also need changing if it ever generates a truly immediate ADR
> (doubtful).
> 

Yes, exactly right.

-Jim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130626/1c484f7e/attachment.html>