[llvm-commits] [PATCH] Fix assembly of narrow Thumb data processing instructions

[Jim Grosbach]

Hi Richard,

This looks great, thank you. You scared me a bit in the description here w/ a reference to validateInstruction(), but the actual patch correctly uses processInstruction(), so all good. :)

Very minor nit: Add periods to the ends of sentences in comments. For one example,
+    // Assemblers should use the narrow encodings of these instructions when permissable
+    // These instructions are special in that they are commutable, so shorter encodings
+    // are available more often

Second, it's best not to use R0 in assembler test cases, as that's the default value for the encoder, so we won't be able to tell the difference between a correct encoding of the operand and the operand not being encoded explicitly at all. There are a lot of tests already in the test suite that are horrible examples in this regard, unfortunately.

Good to commit w/ those fixes.

-Jim

On Jul 4, 2012, at 11:43 AM, Richard Barton <richard.barton at arm.com> wrote:

> Hello Reviewers
> 
> The attached patch implements correct assembly of Thumb dp-operations with both
> three and two register syntaxes.
> 
> i.e. AND, EOR, LSL, LSR, ASR, ADC, SBC, ROR, and ORR.
> 
> The assembly syntax for these instructions is of the form:
> 
> AND{S}{<c>}{<q>}  {<Rd>,} <Rn>, <Rm> {, <shift>}
> 
> The optional Rd argument allows the use of the short encoding when Rd == Rn (and
> other conditions hold.) The assembler should always prefer the short encoding
> over the wide in these cases, unless the wide encoding is specifically requested
> with '.W'.
> 
> In addition, ADD, EOR, ADC and ORR are commutative, so the narrow encoding is
> also available when Rd == Rm (as well as the other conditions holding.)
> 
> The additional conditions are:
> - Rd, Rn and Rm are all in the range R0 - R7.
> - The instruction is flag-setting and not in an IT block, or not-flag setting
> and in an IT block.
> - No shift is applied to the instruction.
> 
> The attached patch uses the validateInstruction callback to chose the narrow
> encodings in the correct circumstances, and adds a new regression test.
> 
> Please review.
> 
> Regards,
> Richard Barton
> ARM Ltd, Cambridge
> 
> <thumb2-narrow-dp-assembly.patch>