[llvm-commits] [PATCH] Fix broken decoding for VCVT (between floating-point and fixed-point, Floating-point)

[Richard Barton]

Hi Jim

> Two things, if you don't mind. First, please add tests for the other
> instructions that are changing, not just vcvt.

I think that my change will only affect vcvt (between floating-point and
fixed-point, Floating-point) instructions for which there are 16 variants
(combinations of f64,u32,etc.) 

> For obvious reasons, not using
> a register numbered zero is a good idea, and one I wish more of our other
> tests followed. Second, much more nitty, please format the tests grouped and
> with the CHECK lines following the instructions being tested (like the tests
> towards the end of the file).

Ah, I see that there are already tests for VCVT at the bottom of the test case.
Ironically, had the register values not been 0, they would have detected this
bug. I have added 12 more so that all 16 flavours are covered, and I tried them
with a few non-zero register values. There should be tests that exercise bit 22
as well.

Assuming I have understood this correctly, the attached patch should cover this.

Thanks,
Rich

> -----Original Message-----
> From: Jim Grosbach [mailto:grosbach at apple.com]
> Sent: 15 March 2012 01:07
> To: Richard Barton
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] Fix broken decoding for VCVT (between
> floating-point and fixed-point, Floating-point)
> 
> Hi Richard,
> 
> This is great. Thanks for doing this.
> 
> Two things, if you don't mind. First, please add tests for the other
> instructions that are changing, not just vcvt. For obvious reasons, not using
> a register numbered zero is a good idea, and one I wish more of our other
> tests followed. Second, much more nitty, please format the tests grouped and
> with the CHECK lines following the instructions being tested (like the tests
> towards the end of the file). That particular file hasn't yet been fully
> converted to that style yet, so I certainly understand the confusion.
> 
> Thanks again, and great bug catch.
> 
> -Jim
> 
> On Mar 9, 2012, at 10:20 AM, Richard Barton wrote:
> 
> > Hello
> >
> > The attached patch fixes VCVT (between floating-point and fixed-point,
> > Floating-point) instruction decoding in MC.
> >
> > The previous support omitted the register operand from the encoding. My
> patch
> > adds a new class to the hierarchy to handle the two different encodings for
> > double-precision and single-precision registers.
> >
> > I have added a regression test for single and double precision flavours of
> this
> > instruction.
> >
> > Thanks,
> > Richard Barton
> >
> >
> <vcvt_floating_to_fixed.patch>_______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vcvt_floating_to_fixed.patch
Type: application/octet-stream
Size: 8691 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120315/c99d7e5c/attachment.obj>