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

[Anton Lokhmotov]

Hi Doug,

> I'm rather unnerved by the prevalence of this pattern in IR generation:
> 
> -    if (type->isHalfType()) {
> +    if (type->isHalfType() && !CGF.getContext().getLangOpts().OpenCL)
> 
> Using the OpenCL language option here seems wrong, especially given
> that we tend to permit OpenCL extensions in other language dialects.

Could you please clarify what you mean by OpenCL extensions being permitted
in other language dialects?

> I think what you really want to be checking for here is whether the
> target wants to use native half types vs. faking them with a storage-
> only int16, per this FIXME you're zapping:
> 	
> -      // FIXME: Ask target which kind of half FP it prefers (storage
> only vs
> -      // native).
> 
> However, that seems like something that would also impact our semantic
> analysis. I don't have a solid understanding of the similarities and
> differences between the OpenCL and ARM NEON's half types, but from your
> patch it's starting to feel like they are conceptually different types.

I'm looking to Anton Korobeynikov here (who I believe left this comment), as
I'm only vaguely familiar with the NEON half semantics.  Do you think we
should have a LangOpt such as HalfTypeNative, which would be always enabled
for OpenCL, with a typical pattern of:

> -    if (type->isHalfType()) {
> +    if (type->isHalfType() &&
!CGF.getContext().getLangOpts().HalfTypeNative)

This would probably be a reasonable refactoring if any language dialect
other than OpenCL needs to use the half type natively.

> There's no need to map down to the canonical type here; isHalfType()
> will do it for you. This would be much cleaner as:
> 
> 	if (R->isHalfType()) {
> 		// diagnose error
> 	} else if (const ArrayType *ArrayTy = R->getAs<ArrayType>()) {
> 		if (ArrayTy->getElementType()->isHalfType()) {
> 			// diagnose error
> 		}
> 	}
> 
Sadly, getAs<ArrayType>() is explicitly forbidden for qualified types.
Please see the updated patch for other improvements.

Many thanks,
Anton.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: half-clang.patch
Type: application/octet-stream
Size: 17390 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120621/100ead51/attachment.obj>