[PATCH] D15120: Add support for __float128 type to be used by targets that support it

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: include/clang/Driver/Options.td:1463
@@ +1462,3 @@
+    Group<m_ppc_Features_Group>;
+def mnofloat128 : Flag<["-"], "mno-float128">,
+    Group<m_ppc_Features_Group>;
----------------
Per the mangling rules at the top of this file, this should be named `mno_float128`.

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3357-3358
@@ -3355,4 +3356,4 @@
       getContext().FloatTy,            getContext().DoubleTy,
       getContext().LongDoubleTy,       getContext().Char16Ty,
-      getContext().Char32Ty,
+      getContext().Char32Ty,           getContext().Float128Ty
   };
----------------
Please keep this in the same order as `TypeInfoIsInStandardLibrary` (put `Float128Ty` after `LongDoubleTy`).

================
Comment at: lib/Sema/SemaExpr.cpp:1156-1159
@@ +1155,6 @@
+
+  QualType LHSElemType = dyn_cast<ComplexType>(LHSType) ?
+    cast<ComplexType>(LHSType)->getElementType() : LHSType;
+  QualType RHSElemType = dyn_cast<ComplexType>(RHSType) ?
+    cast<ComplexType>(RHSType)->getElementType() : RHSType;
+
----------------
hubert.reinterpretcast wrote:
> The result of the `dyn_cast` can be saved instead of using `cast` after the `dyn_cast`.
Do not use `dyn_cast` / `cast` on types, use `LHSType->getAs<...>()` / `LHSType->castAs<...>()` instead. This will do the wrong thing on sugared types (eg, a typedef for a complex type).

================
Comment at: lib/Sema/SemaExpr.cpp:1337
@@ -1303,1 +1336,3 @@
 
+  // Diagnose builtin types that have the same size and different representation
+  if (sameWidthDifferentRepresentation(*this, LHSType, RHSType))
----------------
Please indicate why you're diagnosing these here.

================
Comment at: lib/Sema/SemaExpr.cpp:1339
@@ +1338,3 @@
+  if (sameWidthDifferentRepresentation(*this, LHSType, RHSType))
+      return QualType();
+
----------------
Too much indentation.

================
Comment at: lib/Sema/SemaOverload.cpp:1655-1664
@@ -1654,2 +1654,12 @@
   } else if (FromType->isRealFloatingType() && ToType->isRealFloatingType()) {
+      // FIXME: disable conversions between long double and __float128 if
+      // their representation is different until there is back end support
+      if (&S.Context.getFloatTypeSemantics(FromType) !=
+          &S.Context.getFloatTypeSemantics(ToType)) {
+        if ((FromType == S.Context.Float128Ty &&
+             ToType == S.Context.LongDoubleTy) ||
+            (FromType == S.Context.LongDoubleTy &&
+             ToType == S.Context.Float128Ty))
+          return false;
+      }
     // Floating point conversions (C++ 4.8).
----------------
This is over-indented.


Repository:
  rL LLVM

http://reviews.llvm.org/D15120