[llvm-commits] [PATCH]Add support for fstmx/fldmx instructions on the ARM backend

[Jim Grosbach]

Hi Silviu,

Thanks for the ping. I'm hoping to get to this very soon. WWDC is this week, so my normal schedule is in a bit of disarray.

-Jim
On Jun 11, 2012, at 12:52 AM, Silviu Baranga <silbar01 at arm.com> wrote:

> Ping.
>  
> From: Silviu Baranga [mailto:silbar01 at arm.com] 
> Sent: 24 May 2012 13:22
> To: 'Jim Grosbach'
> Cc: llvm-commits at cs.uiuc.edu
> Subject: RE: [PATCH]Add support for fstmx/fldmx instructions on the ARM backend
>  
> Hi Jim,
>  
> Thanks for the review.
>  
> These instructions also support the “DB” suffix, and having
> either IA/DB is a required part of the syntax.
>  
> The tests from fstmx-arm.txt were supposed to test that
> some invalid bit patterns are not decoded as vstm/vldm
> instructions (which is  the current behaviour).
> Those bit patterns are invalid because they have an odd
> imm8 field (so they are not vstm/vldm instructions) and
> the value of the 22nd bit is 1 (so they are not fstmx/fldmx
> instructions).
>  
> I’ve removed the fstmx-arm.txt test, and added instead
> the invalid-fstmx.txt test file (the new patch is attached).
>  
> Thanks,
> Silviu
>  
> From: Jim Grosbach [mailto:grosbach at apple.com] 
> Sent: 22 May 2012 17:32
> To: Silviu Baranga
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH]Add support for fstmx/fldmx instructions on the ARM backend
>  
> Hi Silviu,
>  
> Do these instructions only support the "IA" suffix? Is that suffix always a required part of the syntax?
>  
> Index: test/MC/Disassembler/ARM/fstmx-arm.txt
> ===================================================================
> --- test/MC/Disassembler/ARM/fstmx-arm.txt         (revision 0)
> +++ test/MC/Disassembler/ARM/fstmx-arm.txt      (revision 0)
> @@ -0,0 +1,9 @@
> +# RUN: echo "0x0d 0x0b 0xc7 0x0c" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x0b 0x5b 0xd2 0x0c" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x07 0x1b 0x69 0x0d" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x09 0xeb 0x37 0x0d" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x0d 0xfb 0xa4 0x0c" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +# RUN: echo "0x0b 0x3b 0xfb 0x0c" | llvm-mc --disassemble -triple=armv7-linux-gnueabi |& FileCheck %s
> +
> +# CHECK-NOT: vstm
> +# CHECK-NOT: vldm
>  
> Why all the separate run lines with 'echo' instead of having the bytes in the file directly, one instruction per line, like the other tests? Additionally, this should be checking what the disassembly should be, not just what it should not be. i.e., CHECK lines as well as CHECK-NOT. Relatedly, what is this testing that the entries in fl-encoding.txt do not cover?
>  
> -Jim
>  
> On May 22, 2012, at 4:05 AM, Silviu Baranga <silbar01 at arm.com> wrote:
>  
> 
> Hi,
>  
> The fstmx/fldmx ARM instructions are currently not implemented in the ARM backend.
> Although these instructions are deprecated, they should still be supported for assembly and
> disassembly purposes.
>  
> The patch adds support for these instructions and also fixes a problem with the
> vstm/vldm instructions which were colliding with the fstmx/fldmx encoding space.
>  
> Please review this patch.
>  
> Thanks,
> Silviu
> <fstmx.diff>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120612/4ccaa546/attachment.html>