[PATCH] OpenCL half support

I've got comments on the code change. The test cases look ok, but I
haven't fully checked the math on the half-values.
I checked with reference to trunk top-of-tree at revision 156617. I
have not compiled the code.

lib/AsmParser/LLLexer.cpp

   Adds support to parse format: 0xH<hexdigits>
      Tha 0xH format should be described in LangRef.html alongside
0xK<hex> and 0xM<hex>

   The code looks good though.

lib/VMCore/AsmWriter.cpp
   The change updates Printing support for half data type, so it uses
   the new special 0xH<hex> form.

   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.)

   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)

   Line 762. Update the comment
       // Some form of long double. These appear as a magic letter identifying

   Before:
    // Some form of long double. These appear as a magic letter identifying
    // the type, then a fixed number of hex digits.
   After:
    // Either half, or some form of long double.
    // These appear as a magic letter identifying the type, then a
    // fixed number of hex digits.

   The rest of this patch to AsmWriter.cpp looks good.

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.

half-llvm.patch (6.33 KB)

looks good here.

Anton, would it be possible to add information to the documentation here:
http://llvm.org/docs/BitCodeFormat.html

Anton, would it be possible to add information to the documentation
here:
LLVM Bitcode File Format — LLVM 16.0.0git documentation

Sure!

Any further comments?

Many thanks,
Anton.

half-doc.patch (657 Bytes)

half-llvm.patch (6.33 KB)

No problem here, looks good to me.

Sure!
Any further comments?

LGTM