[clang] 3f22b47 - Revert "[NFC-I] Remove hack for fp-classification builtins"

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 16 14:03:04 PST 2019


Author: Erich Keane
Date: 2019-12-16T14:01:51-08:00
New Revision: 3f22b4721e6c9859c392d9891411cbc8d9e10c70

URL: https://github.com/llvm/llvm-project/commit/3f22b4721e6c9859c392d9891411cbc8d9e10c70
DIFF: https://github.com/llvm/llvm-project/commit/3f22b4721e6c9859c392d9891411cbc8d9e10c70.diff

LOG: Revert "[NFC-I] Remove hack for fp-classification builtins"

This reverts commit b1e542f302c1ed796ad9f703d4d36e010afcb914.

The original 'hack' didn't chop out fp-16 to double conversions, so
systems that use FP16ConversionIntrinsics end up in IR-CodeGen with an
i16 type isntead of a float type (like PPC64-BE).  The bots noticed
this.

Reverting until I figure out how to fix this

Added: 
    

Modified: 
    clang/include/clang/Basic/Builtins.def
    clang/lib/Sema/SemaChecking.cpp
    clang/test/Sema/crash-invalid-builtin.c

Removed: 
    clang/test/Sema/builtin-fpclassification.c


################################################################################
diff  --git a/clang/include/clang/Basic/Builtins.def b/clang/include/clang/Basic/Builtins.def
index 51d3500df8ae..29dec78e4b96 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.", "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")
+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")
 
 // FP signbit builtins
-BUILTIN(__builtin_signbit, "i.", "Fnct")
+BUILTIN(__builtin_signbit, "i.", "Fnc")
 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 80d8a486e4cf..aff63aef2934 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5796,35 +5796,51 @@ 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
deleted file mode 100644
index 83e248bfd1b5..000000000000
--- a/clang/test/Sema/builtin-fpclassification.c
+++ /dev/null
@@ -1,91 +0,0 @@
-// 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 8f749f7b32bb..1c5221fa40d6 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 {{cannot redeclare builtin function '__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 {{conflicting types for '__builtin_isinf'}} // expected-note {{'__builtin_isinf' is a builtin with type 'int ()'}}


        


More information about the cfe-commits mailing list