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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to