Skip to content

Commit e4f5d55

Browse files
[SYCL] Emit a warning for floating-point size changes after implicit conversions (#6323)
Prior to this changes, we had the following 2 warnings for floating-point precision change: ``` -Wimplicit-float-conversion : (example) implicit conversion loses floating-point precision: 'double' to 'float' -Wdouble-promotion : (example) implicit conversion increases floating-point precision: 'float' to 'double' ``` Both the above warnings were turned off by default and only enabled when the corresponding warning flags were passed. Also, both the warnings were only specific to Clang and were not enabled for SYCL (meaning device code). If the floating point values remained the same on both the source and target semantics after the cast, no warning was emitted. Example: ``` float PrecisionLoss = 1.1; // implicit conversion loses floating-point precision: 'double' to 'float' float SizeChange = 3.0; // In this case, though we have assigned a double value to a 'float' variable, there is techncally no loss of precision and we don't get any warning here. But there is still a change in floating-point-rank (size) here ( from 'double' to 'float') ``` This PR identifies a floating-point size change after an implicit cast and triggers a warning when the -Wimplicit-float-size-conversion flag is passed. It is turned off by default. This warning is enabled for both Clang and SYCL(i.e both host and device code) The only other case this addresses is, if there is a precision loss and `-Wimplicit-float-size-conversion` is passed but `-Wimplicit-float-conversion` is not, we make sure to emit at least a size warning.
1 parent 2ad29fc commit e4f5d55

File tree

8 files changed

+85
-12
lines changed

8 files changed

+85
-12
lines changed

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,11 @@ def ImplicitIntFloatConversion : DiagGroup<"implicit-int-float-conversion",
109109
[ImplicitConstIntFloatConversion]>;
110110
def ObjCSignedCharBoolImplicitFloatConversion :
111111
DiagGroup<"objc-signed-char-bool-implicit-float-conversion">;
112+
def ImplicitFloatSizeConversion :
113+
DiagGroup<"implicit-float-size-conversion">;
112114
def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion",
113115
[ImplicitIntFloatConversion,
114-
ObjCSignedCharBoolImplicitFloatConversion]>;
116+
ObjCSignedCharBoolImplicitFloatConversion, ImplicitFloatSizeConversion]>;
115117
def ImplicitFixedPointConversion : DiagGroup<"implicit-fixed-point-conversion">;
116118

117119
def FloatOverflowConversion : DiagGroup<"float-overflow-conversion">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ def warn_float_compare_literal : Warning<
125125
def warn_double_const_requires_fp64 : Warning<
126126
"double precision constant requires %select{cl_khr_fp64|cl_khr_fp64 and __opencl_c_fp64}0, "
127127
"casting to single precision">;
128+
def warn_imp_float_size_conversion : Warning<
129+
"implicit conversion between floating point types of different sizes">,
130+
InGroup<ImplicitFloatSizeConversion>, DefaultIgnore;
128131
def err_half_const_requires_fp16 : Error<
129132
"half precision constant requires cl_khr_fp16">;
130133

clang/lib/Sema/SemaChecking.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13879,15 +13879,31 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
1387913879
Expr::EvalResult result;
1388013880
if (E->EvaluateAsRValue(result, S.Context)) {
1388113881
// Value might be a float, a float vector, or a float complex.
13882-
if (IsSameFloatAfterCast(result.Val,
13883-
S.Context.getFloatTypeSemantics(QualType(TargetBT, 0)),
13884-
S.Context.getFloatTypeSemantics(QualType(SourceBT, 0))))
13882+
if (IsSameFloatAfterCast(
13883+
result.Val,
13884+
S.Context.getFloatTypeSemantics(QualType(TargetBT, 0)),
13885+
S.Context.getFloatTypeSemantics(QualType(SourceBT, 0)))) {
13886+
if (S.getLangOpts().SYCLIsDevice)
13887+
S.SYCLDiagIfDeviceCode(CC, diag::warn_imp_float_size_conversion);
13888+
else
13889+
DiagnoseImpCast(S, E, T, CC,
13890+
diag::warn_imp_float_size_conversion);
1388513891
return;
13892+
}
1388613893
}
1388713894

1388813895
if (S.SourceMgr.isInSystemMacro(CC))
1388913896
return;
13890-
13897+
// If there is a precision conversion between floating point types when
13898+
// -Wimplicit-float-size-conversion is passed but
13899+
// -Wimplicit-float-conversion is not, make sure we emit at least a size
13900+
// warning.
13901+
if (S.Diags.isIgnored(diag::warn_impcast_float_precision, CC)) {
13902+
if (S.getLangOpts().SYCLIsDevice)
13903+
S.SYCLDiagIfDeviceCode(CC, diag::warn_imp_float_size_conversion);
13904+
else
13905+
DiagnoseImpCast(S, E, T, CC, diag::warn_imp_float_size_conversion);
13906+
}
1389113907
DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_float_precision);
1389213908
}
1389313909
// ... or possibly if we're increasing rank, too

clang/test/Sema/conversion.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ void test13(long double v) {
288288
takes_int(v); // expected-warning {{implicit conversion turns floating-point number into integer}}
289289
takes_long(v); // expected-warning {{implicit conversion turns floating-point number into integer}}
290290
takes_longlong(v); // expected-warning {{implicit conversion turns floating-point number into integer}}
291-
takes_float(v); // expected-warning {{implicit conversion loses floating-point precision}}
291+
takes_float(v); // expected-warning {{implicit conversion loses floating-point precision}}
292292
takes_double(v); // expected-warning {{implicit conversion loses floating-point precision}}
293293
takes_longdouble(v);
294294
}
@@ -430,7 +430,6 @@ void test27(ushort16 constants) {
430430
ushort16 brBias = pairedConstants.s6; // expected-warning {{implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 'ushort16'}}
431431
}
432432

433-
434433
float double2float_test1(double a) {
435434
return a; // expected-warning {{implicit conversion loses floating-point precision: 'double' to 'float'}}
436435
}
@@ -448,3 +447,4 @@ float double2float_test4(double a, float b) {
448447
b -= a; // expected-warning {{implicit conversion when assigning computation result loses floating-point precision: 'double' to 'float'}}
449448
return b;
450449
}
450+
float f = 1.0 / 2.0; // expected-warning {{implicit conversion between floating point types of different sizes}}

clang/test/Sema/ext_vector_casts.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ static void splats(int i, long l, __uint128_t t, float f, double d) {
116116

117117
vf = 1 + vf;
118118
vf = l + vf; // expected-warning {{implicit conversion from 'long' to 'float2' (vector of 2 'float' values) may lose precision}}
119-
vf = 2.0 + vf;
119+
vf = 2.0 + vf; // expected-warning {{implicit conversion between floating point types of different sizes}}
120120
vf = d + vf; // expected-warning {{implicit conversion loses floating-point precision}}
121121
vf = vf + 0xffffffff; // expected-warning {{implicit conversion from 'unsigned int' to 'float2' (vector of 2 'float' values) changes value from 4294967295 to 4294967296}}
122122
vf = vf + 2.1; // expected-warning {{implicit conversion loses floating-point precision}}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify=precision-loss,precision-gain,size-change -Wimplicit-float-conversion -Wdouble-promotion -Wimplicit-float-size-conversion \
2+
// RUN: -triple x86_64-apple-darwin %s
3+
4+
// RUN: %clang_cc1 -fsyntax-only -verify=size-only,precision-gain,size-change -Wdouble-promotion -Wimplicit-float-size-conversion \
5+
// RUN: -triple x86_64-apple-darwin %s
6+
7+
// RUN: %clang_cc1 -fsyntax-only -verify=precision-increase -Wdouble-promotion \
8+
// RUN: -triple x86_64-apple-darwin %s
9+
10+
// RUN: %clang_cc1 -fsyntax-only -verify=precision-loss,size-change -Wimplicit-float-conversion \
11+
// RUN: -triple x86_64-apple-darwin %s
12+
13+
// RUN: %clang_cc1 -fsyntax-only -verify \
14+
// RUN: -triple x86_64-apple-darwin %s
15+
16+
// This test checks that floating point conversion warnings are emitted correctly when used in conjunction.
17+
18+
// expected-no-diagnostics
19+
// size-only-warning@+2 {{implicit conversion between floating point types of different sizes}}
20+
// precision-loss-warning@+1 {{implicit conversion loses floating-point precision: 'double' to 'float'}}
21+
float PrecisionLoss = 1.1;
22+
// precision-increase-warning@+2 {{implicit conversion increases floating-point precision: 'float' to 'double'}}
23+
// precision-gain-warning@+1 {{implicit conversion increases floating-point precision: 'float' to 'double'}}
24+
double PrecisionIncrease = 2.1f;
25+
// size-change-warning@+1 {{implicit conversion between floating point types of different sizes}}
26+
float SizeChange = 3.0;
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// RUN: %clang_cc1 -fsycl-is-device -triple spir64 -internal-isystem %S/Inputs -fsyntax-only -sycl-std=2020 -Wimplicit-float-size-conversion -verify=size-only,always-size %s
2+
// RUN: %clang_cc1 -fsycl-is-device -triple spir64 -internal-isystem %S/Inputs -fsyntax-only -sycl-std=2020 -Wimplicit-float-conversion -verify=always-size,precision-only %s
3+
// RUN: %clang_cc1 -fsycl-is-device -triple spir64 -internal-isystem %S/Inputs -fsyntax-only -sycl-std=2020 -Wimplicit-float-conversion -Wno-implicit-float-size-conversion -verify=prefer-precision %s
4+
// RUN: %clang_cc1 -fsycl-is-device -triple spir64 -internal-isystem %S/Inputs -fsyntax-only -sycl-std=2020 -Wno-implicit-float-conversion -verify %s
5+
6+
// This test checks that floating point conversion warnings are emitted correctly when used in conjunction.
7+
8+
#include "sycl.hpp"
9+
class kernelA;
10+
11+
using namespace cl::sycl;
12+
13+
int main() {
14+
queue q;
15+
// expected-no-diagnostics
16+
// always-size-note@#KernelSingleTaskKernelFuncCall {{called by 'kernel_single_task<kernelA, (lambda}}
17+
q.submit([&](handler &h) {
18+
h.single_task<class kernelA>([=]() {
19+
float s = 1.0; // always-size-warning {{implicit conversion between floating point types of different sizes}}
20+
// prefer-precision-warning@+2 {{implicit conversion loses floating-point precision: 'double' to 'float'}}
21+
// precision-only-warning@+1 {{implicit conversion loses floating-point precision: 'double' to 'float'}}
22+
float d = 2.1; // size-only-warning {{implicit conversion between floating point types of different sizes}}
23+
});
24+
});
25+
return 0;
26+
}

sycl/test/basic_tests/stdcpp_compat.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
// RUN: %clangxx -std=c++14 -fsycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify=expected,cxx14 %s -c -o %t.out
22
// RUN: %clangxx -std=c++14 -fsycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify %s -c -o %t.out -DSYCL_DISABLE_CPP_VERSION_CHECK_WARNING=1
3-
// RUN: %clangxx -std=c++14 -fsycl --no-system-header-prefix=CL/sycl --no-system-header-prefix=sycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify=cxx14,warning_extension,expected %s -c -o %t.out
4-
// RUN: %clangxx -std=c++17 -fsycl --no-system-header-prefix=CL/sycl --no-system-header-prefix=sycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify %s -c -o %t.out
5-
// RUN: %clangxx -std=c++20 -fsycl --no-system-header-prefix=CL/sycl --no-system-header-prefix=sycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify %s -c -o %t.out
6-
// RUN: %clangxx -fsycl --no-system-header-prefix=CL/sycl --no-system-header-prefix=sycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify %s -c -o %t.out
3+
// RUN: %clangxx -std=c++14 -fsycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify=cxx14,warning_extension,expected %s -c -o %t.out
4+
// RUN: %clangxx -std=c++17 -fsycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify %s -c -o %t.out
5+
// RUN: %clangxx -std=c++20 -fsycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify %s -c -o %t.out
6+
// RUN: %clangxx -fsycl -Wall -pedantic -Wno-c99-extensions -Wno-deprecated -fsyntax-only -Xclang -verify %s -c -o %t.out
77

88
// The test checks SYCL headers C++ compiance and that a warning is emitted
99
// when compiling in < C++17 mode.

0 commit comments

Comments
 (0)