[clang] 1ed832e - Reland [NFC-I] Remove hack for fp-classification builtins
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 17 06:58:41 PST 2019
Author: Erich Keane
Date: 2019-12-17T06:58:29-08:00
New Revision: 1ed832e42446ef8c4afe08f980db2e54ac316bf3
URL: https://github.com/llvm/llvm-project/commit/1ed832e42446ef8c4afe08f980db2e54ac316bf3
DIFF: https://github.com/llvm/llvm-project/commit/1ed832e42446ef8c4afe08f980db2e54ac316bf3.diff
LOG: Reland [NFC-I] Remove hack for fp-classification builtins
The FP-classification builtins (__builtin_isfinite, etc) use variadic
packs in the definition file to mean an overload set. Because of that,
floats were converted to doubles, which is incorrect. There WAS a patch
to remove the cast after the fact.
THis patch switches these builtins to just be custom type checking,
calls the implicit conversions for the integer members, and makes sure
the correct L->R casts are put into place, then does type checking like
normal.
A future direction (that wouldn't be NFC) would consider making
conversions for the floating point parameter legal.
Note: The initial patch for this missed that certain systems need to
still convert half to float, since they dont' support that type.
Added:
clang/test/Sema/builtin-fpclassification.c
Modified:
clang/include/clang/Basic/Builtins.def
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/builtin_float.c
clang/test/Sema/crash-invalid-builtin.c
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/Builtins.def b/clang/include/clang/Basic/Builtins.def
index 29dec78e4b96..51d3500df8ae 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -399,15 +399,15 @@ BUILTIN(__builtin_islessgreater , "i.", "Fnct")
BUILTIN(__builtin_isunordered , "i.", "Fnct")
// Unary FP classification
-BUILTIN(__builtin_fpclassify, "iiiiii.", "Fnc")
-BUILTIN(__builtin_isfinite, "i.", "Fnc")
-BUILTIN(__builtin_isinf, "i.", "Fnc")
-BUILTIN(__builtin_isinf_sign, "i.", "Fnc")
-BUILTIN(__builtin_isnan, "i.", "Fnc")
-BUILTIN(__builtin_isnormal, "i.", "Fnc")
+BUILTIN(__builtin_fpclassify, "iiiiii.", "Fnct")
+BUILTIN(__builtin_isfinite, "i.", "Fnct")
+BUILTIN(__builtin_isinf, "i.", "Fnct")
+BUILTIN(__builtin_isinf_sign, "i.", "Fnct")
+BUILTIN(__builtin_isnan, "i.", "Fnct")
+BUILTIN(__builtin_isnormal, "i.", "Fnct")
// FP signbit builtins
-BUILTIN(__builtin_signbit, "i.", "Fnc")
+BUILTIN(__builtin_signbit, "i.", "Fnct")
BUILTIN(__builtin_signbitf, "if", "Fnc")
BUILTIN(__builtin_signbitl, "iLd", "Fnc")
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 910afefffb6d..cc091b27fe56 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5797,51 +5797,41 @@ bool Sema::SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs) {
<< SourceRange(TheCall->getArg(NumArgs)->getBeginLoc(),
(*(TheCall->arg_end() - 1))->getEndLoc());
+ // __builtin_fpclassify is the only case where NumArgs != 1, so we can count
+ // on all preceding parameters just being int. Try all of those.
+ for (unsigned i = 0; i < NumArgs - 1; ++i) {
+ Expr *Arg = TheCall->getArg(i);
+
+ if (Arg->isTypeDependent())
+ return false;
+
+ ExprResult Res = PerformImplicitConversion(Arg, Context.IntTy, AA_Passing);
+
+ if (Res.isInvalid())
+ return true;
+ TheCall->setArg(i, Res.get());
+ }
+
Expr *OrigArg = TheCall->getArg(NumArgs-1);
if (OrigArg->isTypeDependent())
return false;
+ // Usual Unary Conversions will convert half to float, which we want for
+ // machines that use fp16 conversion intrinsics. Else, we wnat to leave the
+ // type how it is, but do normal L->Rvalue conversions.
+ if (Context.getTargetInfo().useFP16ConversionIntrinsics())
+ OrigArg = UsualUnaryConversions(OrigArg).get();
+ else
+ OrigArg = DefaultFunctionArrayLvalueConversion(OrigArg).get();
+ TheCall->setArg(NumArgs - 1, OrigArg);
+
// This operation requires a non-_Complex floating-point number.
if (!OrigArg->getType()->isRealFloatingType())
return Diag(OrigArg->getBeginLoc(),
diag::err_typecheck_call_invalid_unary_fp)
<< OrigArg->getType() << OrigArg->getSourceRange();
- // If this is an implicit conversion from float -> float, double, or
- // long double, or half -> half, float, double, or long double, remove it.
- if (ImplicitCastExpr *Cast = dyn_cast<ImplicitCastExpr>(OrigArg)) {
- // Only remove standard FloatCasts, leaving other casts inplace
- if (Cast->getCastKind() == CK_FloatingCast) {
- bool IgnoreCast = false;
- Expr *CastArg = Cast->getSubExpr();
- if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
- assert(
- (Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) &&
- "promotion from float to either float, double, or long double is "
- "the only expected cast here");
- IgnoreCast = true;
- } else if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Half) &&
- !Context.getTargetInfo().useFP16ConversionIntrinsics()) {
- assert(
- (Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::Half) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) &&
- "promotion from half to either half, float, double, or long double "
- "is the only expected cast here");
- IgnoreCast = true;
- }
-
- if (IgnoreCast) {
- Cast->setSubExpr(nullptr);
- TheCall->setArg(NumArgs-1, CastArg);
- }
- }
- }
-
return false;
}
diff --git a/clang/test/CodeGen/builtin_float.c b/clang/test/CodeGen/builtin_float.c
index b97cf1be348f..b257854278cb 100644
--- a/clang/test/CodeGen/builtin_float.c
+++ b/clang/test/CodeGen/builtin_float.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-windows-pc -o - %s | FileCheck %s --check-prefixes=CHECK,FP16
+// RUN: %clang_cc1 -emit-llvm -triple ppc64-be -o - %s -DNO_FP16 | FileCheck %s --check-prefixes=CHECK,NOFP16
// test to ensure that these builtins don't do the variadic promotion of float->double.
void test_floats(float f1, float f2) {
@@ -43,6 +44,15 @@ void test_doubles(double d1, double f2) {
// CHECK-NEXT: zext i1
}
+void test_half(__fp16 *H, __fp16 *H2) {
+ (void)__builtin_isgreater(*H, *H2);
+ // CHECK: fcmp ogt float
+ // CHECK-NEXT: zext i1
+ (void)__builtin_isinf(*H);
+ // NOFP16: fcmp oeq float %{{.+}}, 0x7FF
+ // FP16: fcmp oeq half %{{.+}}, 0xH7C
+}
+
void test_mixed(double d1, float f2) {
(void)__builtin_isgreater(d1, f2);
// CHECK: fpext float {{.*}} to double
diff --git a/clang/test/Sema/builtin-fpclassification.c b/clang/test/Sema/builtin-fpclassification.c
new file mode 100644
index 000000000000..83e248bfd1b5
--- /dev/null
+++ b/clang/test/Sema/builtin-fpclassification.c
@@ -0,0 +1,91 @@
+// RUN: %clang_cc1 %s -Wno-unused-value -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Wno-unused-value -ast-dump -DAST_CHECK | FileCheck %s
+
+struct S {};
+void usage(float f, int i, double d) {
+#ifdef AST_CHECK
+ __builtin_fpclassify(d, 1, i, i, 3, d);
+ //CHECK: CallExpr
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: <BuiltinFnToFnPtr>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: '__builtin_fpclassify'
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'int' <FloatingToIntegral>
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'double' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'd' 'double'
+ //CHECK-NEXT: IntegerLiteral
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'int' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'i' 'int'
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'int' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'i' 'int'
+ //CHECK-NEXT: IntegerLiteral
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'double' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'd' 'double'
+
+ __builtin_fpclassify(f, 1, i, i, 3, f);
+ //CHECK: CallExpr
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: <BuiltinFnToFnPtr>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: '__builtin_fpclassify'
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'int' <FloatingToIntegral>
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'float' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'f' 'float'
+ //CHECK-NEXT: IntegerLiteral
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'int' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'i' 'int'
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'int' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'i' 'int'
+ //CHECK-NEXT: IntegerLiteral
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'float' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'f' 'float'
+
+ __builtin_isfinite(f);
+ //CHECK: CallExpr
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: <BuiltinFnToFnPtr>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: '__builtin_isfinite'
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'float' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'f' 'float'
+
+ __builtin_isfinite(d);
+ //CHECK: CallExpr
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: <BuiltinFnToFnPtr>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: '__builtin_isfinite'
+ //CHECK-NEXT: ImplicitCastExpr
+ //CHECK-SAME: 'double' <LValueToRValue>
+ //CHECK-NEXT: DeclRefExpr
+ //CHECK-SAME: 'd' 'double'
+#else
+ struct S s;
+ // expected-error at +1{{passing 'struct S' to parameter of incompatible type 'int'}}
+ __builtin_fpclassify(d, s, i, i, 3, d);
+ // expected-error at +1{{floating point classification requires argument of floating point type (passed in 'int')}}
+ __builtin_fpclassify(d, 1, i, i, 3, i);
+ // expected-error at +1{{floating point classification requires argument of floating point type (passed in 'int')}}
+ __builtin_isfinite(i);
+#endif
+}
diff --git a/clang/test/Sema/crash-invalid-builtin.c b/clang/test/Sema/crash-invalid-builtin.c
index 1c5221fa40d6..8f749f7b32bb 100644
--- a/clang/test/Sema/crash-invalid-builtin.c
+++ b/clang/test/Sema/crash-invalid-builtin.c
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -triple=x86_64-apple-darwin -fsyntax-only -verify %s
// PR23086
-__builtin_isinf(...); // expected-warning {{type specifier missing, defaults to 'int'}} expected-error {{ISO C requires a named parameter before '...'}} // expected-error {{conflicting types for '__builtin_isinf'}} // expected-note {{'__builtin_isinf' is a builtin with type 'int ()'}}
+__builtin_isinf(...); // expected-warning {{type specifier missing, defaults to 'int'}} expected-error {{ISO C requires a named parameter before '...'}} // expected-error {{cannot redeclare builtin function '__builtin_isinf'}} // expected-note {{'__builtin_isinf' is a builtin with type 'int ()'}}
More information about the cfe-commits
mailing list