Author: Erich Keane Date: 2019-12-16T12:22:55-08:00 New Revision: b1e542f302c1ed796ad9f703d4d36e010afcb914
URL: https://github.com/llvm/llvm-project/commit/b1e542f302c1ed796ad9f703d4d36e010afcb914 DIFF: https://github.com/llvm/llvm-project/commit/b1e542f302c1ed796ad9f703d4d36e010afcb914.diff LOG: [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. Added: clang/test/Sema/builtin-fpclassification.c Modified: clang/include/clang/Basic/Builtins.def clang/lib/Sema/SemaChecking.cpp 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 aff63aef2934..80d8a486e4cf 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -5796,51 +5796,35 @@ 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; + 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/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@+1{{passing 'struct S' to parameter of incompatible type 'int'}} + __builtin_fpclassify(d, s, i, i, 3, d); + // expected-error@+1{{floating point classification requires argument of floating point type (passed in 'int')}} + __builtin_fpclassify(d, 1, i, i, 3, i); + // expected-error@+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 ()'}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits