r289544 - Improve handling of floating point literals in OpenCL to only use double precision if the target supports fp64.

Tom Stellard via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 13 12:24:04 PST 2016


On Tue, Dec 13, 2016 at 04:22:50PM -0000, Neil Hickey via cfe-commits wrote:
> Author: neil.hickey
> Date: Tue Dec 13 10:22:50 2016
> New Revision: 289544
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=289544&view=rev
> Log:
> Improve handling of floating point literals in OpenCL to only use double precision if the target supports fp64.
> 
> This change makes sure single-precision floating point types are used if the 
> cl_fp64 extension is not supported by the target.
> 
> Also removed the check to see whether the OpenCL version is >= 1.2, as this has
> been incorporated into the extension setting code.
> 
> Differential Revision: https://reviews.llvm.org/D24235
> 
> 

This patch causes a crash when compiling libclc.  You can reproduce by
compiling this function:

int isnan(float x){ return __builtin_isnan(x); }

As I recall there is some weirdness with how clang does type
deduction for these builtins, but the compiler shouldn't be crashing.

-Tom


> Modified:
>     cfe/trunk/lib/Sema/SemaChecking.cpp
>     cfe/trunk/lib/Sema/SemaExpr.cpp
>     cfe/trunk/lib/Sema/SemaType.cpp
>     cfe/trunk/test/CodeGenOpenCL/fpmath.cl
>     cfe/trunk/test/SemaOpenCL/extensions.cl
> 
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=289544&r1=289543&r2=289544&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec 13 10:22:50 2016
> @@ -3748,12 +3748,13 @@ bool Sema::SemaBuiltinFPClassification(C
>                  diag::err_typecheck_call_invalid_unary_fp)
>        << OrigArg->getType() << OrigArg->getSourceRange();
>  
> -  // If this is an implicit conversion from float -> double, remove it.
> +  // If this is an implicit conversion from float -> float or double, remove it.
>    if (ImplicitCastExpr *Cast = dyn_cast<ImplicitCastExpr>(OrigArg)) {
>      Expr *CastArg = Cast->getSubExpr();
>      if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
> -      assert(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) &&
> -             "promotion from float to double is the only expected cast here");
> +        assert((Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
> +                Cast->getType()->isSpecificBuiltinType(BuiltinType::Float)) &&
> +             "promotion from float to either float or double is the only expected cast here");
>        Cast->setSubExpr(nullptr);
>        TheCall->setArg(NumArgs-1, CastArg);
>      }
> 
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=289544&r1=289543&r2=289544&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Dec 13 10:22:50 2016
> @@ -817,8 +817,16 @@ ExprResult Sema::DefaultArgumentPromotio
>    // double.
>    const BuiltinType *BTy = Ty->getAs<BuiltinType>();
>    if (BTy && (BTy->getKind() == BuiltinType::Half ||
> -              BTy->getKind() == BuiltinType::Float))
> -    E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
> +              BTy->getKind() == BuiltinType::Float)) {
> +    if (getLangOpts().OpenCL &&
> +        !(getOpenCLOptions().cl_khr_fp64)) {
> +        if (BTy->getKind() == BuiltinType::Half) {
> +            E = ImpCastExprToType(E, Context.FloatTy, CK_FloatingCast).get();
> +        }
> +    } else {
> +      E = ImpCastExprToType(E, Context.DoubleTy, CK_FloatingCast).get();
> +    }
> +  }
>  
>    // C++ performs lvalue-to-rvalue conversion as a default argument
>    // promotion, even on class types, but note:
> @@ -3397,10 +3405,13 @@ ExprResult Sema::ActOnNumericConstant(co
>  
>      if (Ty == Context.DoubleTy) {
>        if (getLangOpts().SinglePrecisionConstants) {
> -        Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
> +        const BuiltinType *BTy = Ty->getAs<BuiltinType>();
> +        if (BTy->getKind() != BuiltinType::Float) {
> +          Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
> +        }
>        } else if (getLangOpts().OpenCL &&
> -                 !((getLangOpts().OpenCLVersion >= 120) ||
> -                   getOpenCLOptions().cl_khr_fp64)) {
> +                 !(getOpenCLOptions().cl_khr_fp64)) {
> +        // Impose single-precision float type when cl_khr_fp64 is not enabled.
>          Diag(Tok.getLocation(), diag::warn_double_const_requires_fp64);
>          Res = ImpCastExprToType(Res, Context.FloatTy, CK_FloatingCast).get();
>        }
> 
> Modified: cfe/trunk/lib/Sema/SemaType.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=289544&r1=289543&r2=289544&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaType.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaType.cpp Tue Dec 13 10:22:50 2016
> @@ -1403,8 +1403,7 @@ static QualType ConvertDeclSpecToType(Ty
>        Result = Context.DoubleTy;
>  
>      if (S.getLangOpts().OpenCL &&
> -        !((S.getLangOpts().OpenCLVersion >= 120) ||
> -          S.getOpenCLOptions().cl_khr_fp64)) {
> +        !(S.getOpenCLOptions().cl_khr_fp64)) {
>        S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_requires_extension)
>            << Result << "cl_khr_fp64";
>        declarator.setInvalidType(true);
> 
> Modified: cfe/trunk/test/CodeGenOpenCL/fpmath.cl
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/fpmath.cl?rev=289544&r1=289543&r2=289544&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenOpenCL/fpmath.cl (original)
> +++ cfe/trunk/test/CodeGenOpenCL/fpmath.cl Tue Dec 13 10:22:50 2016
> @@ -1,5 +1,7 @@
>  // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown | FileCheck --check-prefix=CHECK --check-prefix=NODIVOPT %s
>  // RUN: %clang_cc1 %s -emit-llvm -o - -triple spir-unknown-unknown -cl-fp32-correctly-rounded-divide-sqrt | FileCheck --check-prefix=CHECK --check-prefix=DIVOPT %s
> +// RUN: %clang_cc1 %s -emit-llvm -o - -DNOFP64 -cl-std=CL1.2 -triple r600-unknown-unknown -target-cpu r600 -pedantic | FileCheck --check-prefix=CHECK-FLT %s
> +// RUN: %clang_cc1 %s -emit-llvm -o - -DFP64 -cl-std=CL1.2 -pedantic | FileCheck --check-prefix=CHECK-DBL %s
>  
>  typedef __attribute__(( ext_vector_type(4) )) float float4;
>  
> @@ -21,14 +23,26 @@ float4 spvectordiv(float4 a, float4 b) {
>    return a / b;
>  }
>  
> -#pragma OPENCL EXTENSION cl_khr_fp64 : enable
> +#if __OPENCL_C_VERSION__ >=120
> +void printf(constant char* fmt, ...);
> +
> +void testdbllit(long *val) {
> +  // CHECK-FLT: float 2.000000e+01
> +  // CHECK-DBL: double 2.000000e+01
> +  printf("%f", 20.0);
> +}
>  
> +#endif
> +
> +#ifndef NOFP64
> +#pragma OPENCL EXTENSION cl_khr_fp64 : enable
>  double dpscalardiv(double a, double b) {
>    // CHECK: @dpscalardiv
>    // CHECK: #[[ATTR]]
>    // CHECK-NOT: !fpmath
>    return a / b;
>  }
> +#endif
>  
>  // CHECK: attributes #[[ATTR]] = {
>  // NODIVOPT: "correctly-rounded-divide-sqrt-fp-math"="false"
> 
> Modified: cfe/trunk/test/SemaOpenCL/extensions.cl
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/extensions.cl?rev=289544&r1=289543&r2=289544&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaOpenCL/extensions.cl (original)
> +++ cfe/trunk/test/SemaOpenCL/extensions.cl Tue Dec 13 10:22:50 2016
> @@ -1,5 +1,6 @@
>  // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only
>  // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL1.1
> +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL1.2 -DFP64
>  
>  // Test with a target not supporting fp64.
>  // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64 -DNOFP16
> @@ -23,10 +24,16 @@
>  
>  
>  
> +#ifdef FP64
> +// expected-no-diagnostics
> +#endif
> +
> +#if __OPENCL_C_VERSION__ < 120
>  void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}}
>    double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
>    (void) 1.0; // expected-warning {{double precision constant requires cl_khr_fp64}}
>  }
> +#endif
>  
>  #pragma OPENCL EXTENSION cl_khr_fp64 : enable
>  #ifdef NOFP64
> @@ -45,8 +52,9 @@ void f2(void) {
>  #endif
>  
>    (void) 1.0;
> +
>  #ifdef NOFP64
> -// expected-warning at -2{{double precision constant requires cl_khr_fp64, casting to single precision}}
> +// expected-warning at -3{{double precision constant requires cl_khr_fp64, casting to single precision}}
>  #endif
>  }
>  
> @@ -55,6 +63,8 @@ void f2(void) {
>  // expected-warning at -2{{unsupported OpenCL extension 'cl_khr_fp64' - ignoring}}
>  #endif
>  
> +#if __OPENCL_C_VERSION__ < 120
>  void f3(void) {
>    double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
>  }
> +#endif
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


More information about the cfe-commits mailing list