r321301 - [AArch64] Enable fp16 data type for the Builtin for AArch64 only.

[Ahmed Bougacha via cfe-commits]

On Thu, Dec 21, 2017 at 12:10 PM, Abderrazek Zaafrani via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: az
> Date: Thu Dec 21 12:10:03 2017
> New Revision: 321301
>
> URL: http://llvm.org/viewvc/llvm-project?rev=321301&view=rev
> Log:
> [AArch64] Enable fp16 data type for the Builtin for AArch64 only.
>
> Differential Revision: https:://reviews.llvm.org/D41360
>
> Modified:
>     cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>     cfe/trunk/lib/CodeGen/CodeGenFunction.h
>     cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=321301&r1=321300&r2=321301&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Dec 21 12:10:03 2017
> @@ -3334,10 +3334,10 @@ static Value *EmitTargetArchBuiltinExpr(
>    case llvm::Triple::armeb:
>    case llvm::Triple::thumb:
>    case llvm::Triple::thumbeb:
> -    return CGF->EmitARMBuiltinExpr(BuiltinID, E);
> +    return CGF->EmitARMBuiltinExpr(BuiltinID, E, Arch);
>    case llvm::Triple::aarch64:
>    case llvm::Triple::aarch64_be:
> -    return CGF->EmitAArch64BuiltinExpr(BuiltinID, E);
> +    return CGF->EmitAArch64BuiltinExpr(BuiltinID, E, Arch);
>    case llvm::Triple::x86:
>    case llvm::Triple::x86_64:
>      return CGF->EmitX86BuiltinExpr(BuiltinID, E);
> @@ -3378,6 +3378,7 @@ Value *CodeGenFunction::EmitTargetBuilti
>
>  static llvm::VectorType *GetNeonType(CodeGenFunction *CGF,
>                                       NeonTypeFlags TypeFlags,
> +                                     llvm::Triple::ArchType Arch,
>                                       bool V1Ty=false) {
>    int IsQuad = TypeFlags.isQuad();
>    switch (TypeFlags.getEltType()) {
> @@ -3388,7 +3389,12 @@ static llvm::VectorType *GetNeonType(Cod
>    case NeonTypeFlags::Poly16:
>      return llvm::VectorType::get(CGF->Int16Ty, V1Ty ? 1 : (4 << IsQuad));
>    case NeonTypeFlags::Float16:
> -    return llvm::VectorType::get(CGF->HalfTy, V1Ty ? 1 : (4 << IsQuad));
> +    // FIXME: Only AArch64 backend can so far properly handle half types.
> +    // Remove else part once ARM backend support for half is complete.
> +    if (Arch == llvm::Triple::aarch64)
> +      return llvm::VectorType::get(CGF->HalfTy, V1Ty ? 1 : (4 << IsQuad));
> +    else
> +      return llvm::VectorType::get(CGF->Int16Ty, V1Ty ? 1 : (4 << IsQuad));

Hey Abderrazek,

I don't think you need to pass 'Arch' around.  You're already in CGF,
which has a triple:
  CGF->getTarget().getTriple().getArch()
It is slightly different from what's passed in EmitTargetArchBuiltinExpr:
   CGF->getContext().getAuxTargetInfo()->getTriple().getArch()

though IIRC the 'aux target' is only different for GPU target offloading.

More importantly, should you check for:
  LangOpts.HalfArgsAndReturns
and:
  LangOpts.NativeHalfType
instead?

I think that mirrors the actual requirement.   -fnative-half-type is
disabled by default on ARM.  And on AArch64, always picking 'half'
makes -fnative-half-type mandatory.  I don't remember the
ramifications off the top of my head, but that seems like a larger
discussion to be had.

Thanks for taking a look!
-Ahmed