[cfe-dev] [OpenCL patch] Half type as native when OpenCL's cl_khr_fp16 extension is enabled

[David Neto]

I've reviewed the patch against trunk top-of-tree (revision 156617).
I have not compiled the code.

Generally the code changes look good.  I would like to see some test
cases for code generation updates:
- half constant generation
- conversion to and from half, with other type being "float"
- ++ and --
I understand that the code generation cases may depend on inclusion of
your half-llvm.patch
(http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-April/049117.html)


I have one detailed comment:
The following part of the patch seems unnecessary.
I think the existing code should work because the HalfTypeID should be
lower than the other floating types?

   @@ -641,7 +641,10 @@ Value
*ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
      } else {
        assert(SrcTy->isFloatingPointTy() && DstTy->isFloatingPointTy() &&
               "Unknown real conversion");
   -    if (DstTy->getTypeID() < SrcTy->getTypeID())
   +    int DstId = DstTy->getTypeID();
   +    int SrcId = Src->getType()->getTypeID();
   +    if ((SrcId != llvm::Type::HalfTyID && DstId < SrcId) ||
   +        DstId == llvm::Type::HalfTyID)
          Res = Builder.CreateFPTrunc(Src, DstTy, "conv");
        else
          Res = Builder.CreateFPExt(Src, DstTy, "conv");

thanks,
david

On Wed, May 2, 2012 at 8:57 AM, Anton Lokhmotov <Anton.Lokhmotov at arm.com> wrote:
> Please review an update to the half patch that fixes existing test cases.
>
> Thanks,
> Anton.
>
>
>> -----Original Message-----
>> From: Anton Lokhmotov [mailto:Anton.Lokhmotov at arm.com]
>> Sent: 23 April 2012 15:52
>> To: cfe-dev at cs.uiuc.edu
>> Cc: 'Benyei, Guy'; 'Anton Korobeynikov'; 'peter at pcc.me.uk'
>> Subject: [OpenCL patch] Half type as native when OpenCL's cl_khr_fp16
>> extension is enabled
>>
>> Hi,
>>
>> Please review a patch for mapping the half data type to the native LLVM
>> representation [1] when the cl_khr_fp16 extension is enabled in an
>> OpenCL program.  A new test case is attached.
>>
>> Many thanks,
>> Anton.
>>
>> [1] http://llvm.org/viewvc/llvm-project?view=rev&revision=146786
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>