[LLVMdev] [PATCH] OpenCL half support

[Villmow, Micah]

looks good here.

> -----Original Message-----
> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu]
> On Behalf Of Anton Lokhmotov
> Sent: Thursday, May 17, 2012 4:51 AM
> To: 'David Neto'
> Cc: llvmdev at cs.uiuc.edu
> Subject: Re: [LLVMdev] [PATCH] OpenCL half support
> 
> Hi David,
> 
> Many thanks for the comments!
> 
> >       Tha 0xH format should be described in LangRef.html alongside
> > 0xK<hex> and 0xM<hex>
> Done.
> 
> >    Declaration of "int shiftcount" should be moved to smallest nesting
> >    possible, right after "if ( const ConstantFP ..." at line 710
> >
> >    (The code makes a lot more sense with a good comment on the
> > definition
> >    of shiftcount.  I prefer to add this comment:
> >       int shiftcount; // bit position, in the current word, of the
> > next nibble to print.
> >    This is a problem with the original code, not the patch.)
> Done.
> 
> >    The patch removes half case from the code for single and double.
> >    So you should remove the "bool isHalf" variable in that spot. (line
> > 720)
> I'm not sure I get it.  This variable is still needed couple of lines
> below:
> 
>       bool isHalf = &CFP-
> >getValueAPF().getSemantics()==&APFloat::IEEEhalf;
>       bool isDouble =
> &CFP->getValueAPF().getSemantics()==&APFloat::IEEEdouble;
>       bool isInf = CFP->getValueAPF().isInfinity();
>       bool isNaN = CFP->getValueAPF().isNaN();
>       if (!isHalf && !isInf && !isNaN) {
> 
> >     // Either half, or some form of long double.
> >     // These appear as a magic letter identifying the type, then a
> >     // fixed number of hex digits.
> Done.
> 
> Does it look better to you?
> 
> Many thanks,
> Anton.