r286815 - Improve handling of floating point literals in OpenCL to only use double precision if the target supports fp64.
Neil Hickey via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 13 08:41:41 PST 2016
I looked into this, and for C, clang converts the literal to inf in the first case.
OpenCL spec doesn't mention anything different so I presume the behaviour should match Cs behaviour, which is does with this change.
More exhaustive tests could be implemented to ensure this though.
Neil
> -----Original Message-----
> From: scanon at apple.com [mailto:scanon at apple.com]
> Sent: 15 November 2016 13:20
> To: Neil Hickey
> Cc: cfe-commits at lists.llvm.org; nd
> Subject: Re: r286815 - Improve handling of floating point literals in OpenCL to
> only use double precision if the target supports fp64.
>
> 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