hubert.reinterpretcast added inline comments.
================
Comment at: lib/CodeGen/CGDebugInfo.cpp:550
@@ -548,1 +549,3 @@
case BuiltinType::Double:
+ // FIXME: For targets where lond double and __float128 have the same size,
+ // they are currently indistinguishable in the debugger withouth some
----------------
There are various typos in this comment.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1809
@@ +1808,3 @@
+ else if (value->getType()->isHalfTy())
+ FS = &CGF.getTarget().getHalfFormat();
+ F.convert(*FS, llvm::APFloat::rmTowardZero, &ignored);
----------------
It might make sense to move the call to `getLongDoubleFormat()` to the if/else
block and perhaps to order it as long double/__float128/half.
================
Comment at: lib/Sema/SemaExpr.cpp:1115
@@ -1114,3 +1114,3 @@
int order = S.Context.getFloatingTypeOrder(LHSType, RHSType);
- if (order > 0) {
+ if (order >= 0) {
RHS = S.ImpCastExprToType(RHS.get(), LHSType, CK_FloatingCast);
----------------
Are we all comfortable that if `long double` and `__float128` have the same
representation, but are considered distinct types, then the LHS type is used?
Is this consistent with GCC?
================
Comment at: lib/Sema/SemaExpr.cpp:1146
@@ +1145,3 @@
+ QualType RHSType) {
+ if (!LHSType.getTypePtr()->isFloatingType() ||
+ !RHSType.getTypePtr()->isFloatingType() ||
----------------
There's no actual need for the `.getTypePtr()` part; the overloaded
`operator->` does the same thing.
================
Comment at: lib/Sema/SemaExpr.cpp:1153
@@ +1152,3 @@
+ QualType LHSElemType = LHSType->isComplexType() ?
+ LHSComplexType->getElementType() : LHSType;
+ QualType RHSElemType = RHSType->isComplexType() ?
----------------
`LHSComplexType` is only used here. It could be replaced with
`cast<ComplexType>(LHSType.getTypePtr())` (and similarly for the RHS).
Indeed, since we are checking against specific real element types anyway,
`dyn_cast<ComplexType>(LHSType.getTypePtr())` (and checking for null) would
suffice (no need to use `isComplexType()`).
================
Comment at: test/Sema/float128-ld-incompatibility.cpp:32
@@ +31,2 @@
+ q / ld; // expected-error {{invalid operands to binary expression
('__float128' and 'long double')}}
+}
----------------
I believe a test for the conditional operator would be appropriate.
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
