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

[Nemanja Ivanovic via cfe-commits]

nemanjai added inline comments.

================
Comment at: lib/AST/ItaniumMangle.cpp:2064
@@ +2063,3 @@
+    if (getASTContext().getTargetInfo().useFloat128ManglingForLongDouble())
+      Out << "U10__float128"; // Match the GCC mangling
+    else
----------------
Please refer to https://gcc.gnu.org/ml/gcc-patches/2015-10/txt5mF4d1XnFb.txt.
GCC recently added this for PowerPC and the new mangling was provided for the __float128 type when it coexists with long double.

================
Comment at: lib/AST/StmtPrinter.cpp:1234
@@ -1233,2 +1233,3 @@
   case BuiltinType::LongDouble: OS << 'L'; break;
+  case BuiltinType::Float128:   OS << 'Q'; break;
   }
----------------
As in GCC, __float128 literals in source code have a suffix 'q'/'Q'. I'll be perfectly honest - I don't know when this function is called, but it seems that for consistency, we should use the same suffix here.

================
Comment at: lib/Basic/TargetInfo.cpp:228
@@ -226,1 +227,3 @@
       return LongDouble;
+    if (hasFloat128Type())
+      return Float128;
----------------
Left this condition in but moved it down. This way if a target exists that has 'long double' that is narrower than 128 bits, but it supports __float128, we can still return a 128-bit floating point type here.

================
Comment at: lib/Basic/Targets.cpp:1043
@@ +1042,3 @@
+  bool hasFloat128Type() const override {
+    return HasFloat128;
+  }
----------------
Now that we have different mangling for __float128 and long double, it should be safe to support __float128 and leave the mangling of long double unchanged.

================
Comment at: lib/Basic/Targets.cpp:1236
@@ -1229,1 +1235,3 @@
+  if (HasFloat128)
+    Builder.defineMacro("__FLOAT128__");
 
----------------
GCC defines this macro when -mfloat128 is specified.

================
Comment at: lib/CodeGen/CGExprScalar.cpp:1799
@@ -1798,3 +1798,3 @@
     else {
-      // Remaining types are either Half or LongDouble.  Convert from float.
+      // Remaining types are Half, LongDouble or __float128. Convert from float.
       llvm::APFloat F(static_cast<float>(amount));
----------------
An expression incrementing a variable of __float128 type was added to the test case.

================
Comment at: lib/Frontend/InitPreprocessor.cpp:718
@@ -717,1 +717,3 @@
+  if (TI.hasFloat128Type())
+    DefineFloatMacros(Builder, "FLT128", &TI.getFloat128Format(), "Q");
 
----------------
GCC does not do this at this particular time, but it seems logical to actually provide this macro on targets that support it. For PPC, the macros are defined (and tested) to match IEEE quad precision values (when -mfloat128 is specified).

================
Comment at: lib/Index/USRGeneration.cpp:603
@@ -602,1 +602,3 @@
+        case BuiltinType::Float128:
+          c = 'Q'; break;
         case BuiltinType::NullPtr:
----------------
As far as I can tell, this does not collide and it is consistent with the literal suffix for this type. However, I'd be happy to change it if there is a better suffix to use. Do you have a different suggestion @akyrtzi?


Repository:
  rL LLVM

http://reviews.llvm.org/D15120