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

Stephen Canon via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 15 05:20:12 PST 2016


The reason these commits bring it up is that I don’t see it clearly documented whether:

	float x = 340282356779733661637539395458142568447.0;

is interpreted as

	float x = 340282356779733661637539395458142568447.0f; // x is FLT_MAX

or as

	float x = (float)(340282356779733661637539395458142568447.0); // x is INFINITY

when the no-fp64 mode is active.  I assume that only one of these is the correct behavior, and we should have a regression test that will flag the error if it changes.

– Steve

> On Nov 15, 2016, at 4:51 AM, Neil Hickey <Neil.Hickey at arm.com> wrote:
> 
> It would be valuable to perform that test, if clang tests don't already do this, though I'm not sure it should be part of this particular commit as all this commit should do is change the casts if the target doesn't support double.
> 
> Neil
> 
>> -----Original Message-----
>> From: scanon at apple.com [mailto:scanon at apple.com]
>> Sent: 14 November 2016 16:01
>> To: Neil Hickey
>> Cc: cfe-commits at lists.llvm.org
>> Subject: Re: r286815 - Improve handling of floating point literals in OpenCL to
>> only use double precision if the target supports fp64.
>> 
>> Can you add some non-trivial test cases that exercise double-rounding,
>> especially near the overflow boundary?
>> 
>> e.g. What is the expected value of x if the target does not support fp64?:
>> 
>> 	float x = 340282356779733661637539395458142568447.0;
>> 
>> – Steve
>> 
>>> On Nov 14, 2016, at 6:15 AM, Neil Hickey via cfe-commits <cfe-
>> commits at lists.llvm.org> wrote:
>>> 
>>> Author: neil.hickey
>>> Date: Mon Nov 14 05:15:51 2016
>>> New Revision: 286815
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=286815&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
>>> 
>>> 
>>> Modified:
>>>  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/SemaExpr.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?re
>>> v=286815&r1=286814&r2=286815&view=diff
>>> 
>> ==========================================================
>> ============
>>> ========
>>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Nov 14 05:15:51 2016
>>> @@ -705,9 +705,13 @@ ExprResult Sema::DefaultLvalueConversion  if
>>> (getLangOpts().ObjCAutoRefCount &&
>>>     E->getType().getObjCLifetime() == Qualifiers::OCL_Weak)
>>>   Cleanup.setExprNeedsCleanups(true);
>>> +
>>> +  ExprResult Res = E;
>>> 
>>> -  ExprResult Res = ImplicitCastExpr::Create(Context, T,
>> CK_LValueToRValue, E,
>>> -                                            nullptr, VK_RValue);
>>> +  if ( T != E->getType()) {
>>> +    Res = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue, E,
>>> +                                   nullptr, VK_RValue);  }
>>> 
>>> // C11 6.3.2.1p2:
>>> //   ... if the lvalue has atomic type, the value has the non-atomic version
>>> @@ -817,8 +821,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 +3409,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?re
>>> v=286815&r1=286814&r2=286815&view=diff
>>> 
>> ==========================================================
>> ============
>>> ========
>>> --- cfe/trunk/lib/Sema/SemaType.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaType.cpp Mon Nov 14 05:15:51 2016
>>> @@ -1402,8 +1402,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/fpmat
>>> h.cl?rev=286815&r1=286814&r2=286815&view=diff
>>> 
>> ==========================================================
>> ============
>>> ========
>>> --- cfe/trunk/test/CodeGenOpenCL/fpmath.cl (original)
>>> +++ cfe/trunk/test/CodeGenOpenCL/fpmath.cl Mon Nov 14 05:15:51 2016
>>> @@ -1,5 +1,6 @@
>>> // 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.1 -triple
>>> +r600-unknown-unknown -target-cpu r600 -pedantic | FileCheck
>>> +--check-prefix=CHECK-DBL %s
>>> 
>>> typedef __attribute__(( ext_vector_type(4) )) float float4;
>>> 
>>> @@ -21,14 +22,26 @@ float4 spvectordiv(float4 a, float4 b) {  return a
>>> / b; }
>>> 
>>> +void printf(constant char* fmt, ...);
>>> +
>>> +#ifndef NOFP64
>>> #pragma OPENCL EXTENSION cl_khr_fp64 : enable
>>> +#endif
>>> +void testdbllit(long *val) {
>>> +  // CHECK-DBL: float 2.000000e+01
>>> +  // CHECK: double 2.000000e+01
>>> +  printf("%f", 20.0);
>>> +}
>>> 
>>> +#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/extensio
>>> ns.cl?rev=286815&r1=286814&r2=286815&view=diff
>>> 
>> ==========================================================
>> ============
>>> ========
>>> --- cfe/trunk/test/SemaOpenCL/extensions.cl (original)
>>> +++ cfe/trunk/test/SemaOpenCL/extensions.cl Mon Nov 14 05:15:51 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 @@ -21,12 +22,16 @@
>>> // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic
>>> -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64 -cl-ext=+cl_khr_fp16
>>> -cl-ext=-cl_khr_fp64 -DNOFP64 // RUN: %clang_cc1 %s -triple
>>> spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all
>>> -cl-ext=+cl_khr_fp64,-cl_khr_fp64,+cl_khr_fp16 -DNOFP64
>>> 
>>> +#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
>>> +50,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 -6{{double precision constant requires
>>> +cl_khr_fp64, casting to single precision}}
>>> #endif
>>> }
>>> 
>>> @@ -55,6 +61,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