nemanjai added inline comments.

================
Comment at: lib/Basic/TargetInfo.cpp:231
@@ +230,3 @@
+    if (hasFloat128Type() &&
+        &getFloat128Format() == &llvm::APFloat::IEEEquad)
+      return Float128;
----------------
hubert.reinterpretcast wrote:
> Is it necessary to check that `__float128` is IEEE quad here? Unlike the long 
> double cases, `__float128` really better be 128 bits.
It would indeed be weird if it wasn't so. I can remove the second condition in 
that if.

================
Comment at: lib/Sema/SemaExpr.cpp:1169
@@ +1168,3 @@
+  if (LHSElemType == S.Context.Float128Ty &&
+      RHSElemType == S.Context.LongDoubleTy)
+    return true;
----------------
hubert.reinterpretcast wrote:
> I do not believe that the width of `long double` is sufficiently established 
> to be the same as that of `__float128` in this context.
This would certainly cause any attempts to convert between some different 
implementation of **`long double` **(i.e. the Intel 80-bit type). I believe 
that this behaviour is desired in all cases where **`long double`** and 
**`double`** have a different representation. Namely, we do not currently have 
any support for converting between **`fp128`** and anything that has precision 
>= **`double`** that is not actually **`double`**.

What I propose to do here and in other locations where we diagnose conversions 
between the two types is that the check is:
- One type is `__float128`
- The other is `long double`
- The representation of `long double` is not `llvm::APFloat::IEEEdouble`

Basically in this function, the early exit path would be:
- representations are the same or
- representation of `long double` is `llvm::APFloat::IEEEdouble`

================
Comment at: lib/Sema/SemaExpr.cpp:1341
@@ +1340,3 @@
+  // Diagnose builtin types that have the same size and different 
representation
+  // as conversions between such type currently can't be handled.
+  if (sameWidthDifferentRepresentation(*this, LHSType, RHSType))
----------------
hubert.reinterpretcast wrote:
> s/such type/such types;
Oops, I dropped the s. I'll fix it.

================
Comment at: lib/Sema/SemaOverload.cpp:1655
@@ -1654,1 +1654,3 @@
   } 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
----------------
hubert.reinterpretcast wrote:
> Is conversion between `long double` and `__float128` the correct 
> characterization of the missing back-end support? (as opposed to conversion 
> between `PPCDoubleDouble` and `IEEEquad`)
Well, the same issue exists with `x86_fp80`. We don't have libcalls set up for 
handling that either. Of course the intended semantics and the comment are 
still not quite correct here. Really, what I think we should be after is 
catching attempts to convert between `__float128` and `long double` on targets 
where `long double` is not actually just `double`.


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