rsmith added a comment. +rjmccall for `@encode` and USR mangling.
================ Comment at: include/clang-c/Index.h:2879-2885 @@ -2878,8 +2878,9 @@ CXType_LongDouble = 23, - CXType_NullPtr = 24, - CXType_Overload = 25, - CXType_Dependent = 26, - CXType_ObjCId = 27, - CXType_ObjCClass = 28, - CXType_ObjCSel = 29, + CXType_Float128 = 24, + CXType_NullPtr = 25, + CXType_Overload = 26, + CXType_Dependent = 27, + CXType_ObjCId = 28, + CXType_ObjCClass = 29, + CXType_ObjCSel = 30, CXType_FirstBuiltin = CXType_Void, ---------------- Do not renumber these, the libclang ABI has stability guarantees. Add your new enumerator with value 30 instead. ================ Comment at: include/clang/Analysis/Analyses/ThreadSafetyTIL.h:244-245 @@ -243,2 +243,4 @@ } +// FIXME: does this need a __float128 specialization as well? +// Presumably not since most targets don't support it. ---------------- I don't know, and this template looks suspicious already. We likely shouldn't be assuming that `long double`'s size is 128 in the previous function. ================ Comment at: include/clang/Basic/TokenKinds.def:260 @@ -259,2 +259,3 @@ KEYWORD(double , KEYALL) +KEYWORD(__float128 , KEYALL) KEYWORD(else , KEYALL) ---------------- Please keep the list in alphabetical order, and in any case add this to the GNU extensions list below, not here. ================ Comment at: include/clang/Serialization/ASTBitCodes.h:813 @@ +812,3 @@ + PREDEF_TYPE_OMP_ARRAY_SECTION = 55, + /// \brief The IEEE 754R 128-bit floating point type + PREDEF_TYPE_FLOAT128_ID = 56 ---------------- I would say "The '__float128' type." here -- we don't need to assume that it's *this* 128-bit floating point type on any particular platform (except in the setup for that particular target). ================ Comment at: lib/AST/ASTContext.cpp:972 @@ -971,2 +971,3 @@ TypeDecl *ASTContext::getFloat128StubType() const { + // FIXME: Is the assert (and indeed the function) needed? assert(LangOpts.CPlusPlus && "should only be called for c++"); ---------------- This function only existed to allow us to parse libstdc++'s `numeric_limits` specialization for `__float128`. We won't need it any more once we have real support for `__float128`. ================ Comment at: lib/AST/ASTContext.cpp:8031 @@ -8015,2 +8030,3 @@ break; + // FIXME: we will probably need to handle __float128 case 'd': ---------------- We can defer that until we want to add a builtin that takes a `__float128`. ================ Comment at: lib/AST/ItaniumMangle.cpp:2061 @@ -2060,1 +2060,3 @@ break; + // FIXME: is this the right thing to do (always)? + case BuiltinType::Float128: ---------------- We should test a platform that uses `useFloat128ManglingForLongDouble`. Presumably they must either not support `__float128` or use a different mangling here. (IIRC, they can't make `long double` and `__float128` the same type because that would break libstdc++.) ================ Comment at: lib/AST/MicrosoftMangle.cpp:1708 @@ -1707,1 +1707,3 @@ + // FIXME: We probably want to bail on this for now. + case BuiltinType::Float128: ---------------- Yes, that seems like the best thing to do for now; you can just drop this FIXME. ================ Comment at: lib/AST/NSAPI.cpp:444-445 @@ -443,2 +443,4 @@ case BuiltinType::UInt128: + // FIXME: Added for completeness. Not sure if it's needed. + case BuiltinType::Float128: case BuiltinType::NullPtr: ---------------- This looks appropriate. ================ Comment at: lib/AST/StmtPrinter.cpp:1212-1213 @@ -1211,2 +1211,4 @@ case BuiltinType::LongDouble: OS << 'L'; break; + // FIXME: Need to figure out what this should be + case BuiltinType::Float128: OS << 'Q'; break; } ---------------- Does GCC support a suffix for `__int128`? If not, just add a FIXME like we do for `Half`. This case -- presumably -- should never happen if there are no literals of this type. ================ Comment at: lib/Basic/TargetInfo.cpp:225-228 @@ -223,2 +224,6 @@ case 128: + // FIXME: Return __float128 or long double on targets that support both? + // This placement of the condition will favour the former. + if (hasFloat128Type()) + return Float128; if (&getLongDoubleFormat() == &llvm::APFloat::PPCDoubleDouble || ---------------- Return `long double`. We should prefer the standard type over the GNU extension here. ================ Comment at: lib/Basic/Targets.cpp:1037-1044 @@ -1035,7 +1036,10 @@ bool useFloat128ManglingForLongDouble() const override { return LongDoubleWidth == 128 && LongDoubleFormat == &llvm::APFloat::PPCDoubleDouble && getTriple().isOSBinFormatELF(); } + bool hasFloat128Type() const override { + return HasFloat128; + } }; ---------------- Should we really support both `__float128` and using the `__float128` mangling for `long double` simultaneously? That seems likely to be broken unless there's another mangling for `__float128` that we can use. ================ Comment at: lib/CodeGen/CGExprScalar.cpp:1799 @@ -1798,2 +1798,3 @@ else { + // FIXME: does this need handling of __float128? // Remaining types are either Half or LongDouble. Convert from float. ---------------- Yes. You should be able to test this by incrementing / decrementing a `__float128`. ================ Comment at: lib/Frontend/InitPreprocessor.cpp:717-718 @@ -716,2 +716,4 @@ DefineFloatMacros(Builder, "LDBL", &TI.getLongDoubleFormat(), "L"); + // FIXME: we may need something like + //DefineFloatMacros(Builder, "FLT128", &TI.getFloat128Format(), "Q"); ---------------- What macros does GCC define for `__float128`? ================ Comment at: lib/Sema/SemaLookup.cpp:665-672 @@ -664,9 +664,10 @@ if (II) { if (S.getLangOpts().CPlusPlus11 && S.getLangOpts().GNUMode && II == S.getFloat128Identifier()) { // libstdc++4.7's type_traits expects type __float128 to exist, so // insert a dummy type to make that header build in gnu++11 mode. + // FIXME: Is this still needed since __float128 becomes a built-in type? R.addDecl(S.getASTContext().getFloat128StubType()); return true; } if (S.getLangOpts().CPlusPlus && NameKind == Sema::LookupOrdinaryName && ---------------- Delete this entire `if` block. ================ Comment at: lib/Sema/SemaOverload.cpp:1907-1908 @@ -1906,4 +1906,4 @@ // C99 6.3.1.5p1: // When a float is promoted to double or long double, or a // double is promoted to long double [...]. if (!getLangOpts().CPlusPlus && ---------------- This should also allow promoting `long double` to `__float128`. ================ Comment at: lib/Sema/SemaType.cpp:6696-6702 @@ -6688,8 +6695,9 @@ + // FIXME: Is this check still needed since __float128 becomes a built-in type? // We have an incomplete type. Produce a diagnostic. if (Ident___float128 && T == Context.getTypeDeclType(Context.getFloat128StubType())) { Diag(Loc, diag::err_typecheck_decl_incomplete_type___float128); return true; } ---------------- This isn't necessary any more; remove it. Repository: rL LLVM http://reviews.llvm.org/D15120 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits