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

[Jim Grosbach]

LGTM. Do you have commit access?

-Jim

On Mar 15, 2012, at 4:27 AM, Richard Barton <richard.barton at arm.com> wrote:

> 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
>> 
> <vcvt_floating_to_fixed.patch>