rjmccall added inline comments.

================
Comment at: lib/CodeGen/CGExprScalar.cpp:1016
+  if (DstScale > SrcScale) {
+    // Need to allocate space before shifting left
+    ResultWidth = SrcWidth + DstScale - SrcScale;
----------------
In IR, this isn't really "allocating" space.


================
Comment at: lib/CodeGen/CGExprScalar.cpp:1034
+  if (DstFPSema.isSaturated() &&
+      (CGF.getContext().getCorrespondingSaturatedType(SrcTy) != DstTy)) {
+    auto Mask = APInt::getBitsSetFrom(
----------------
Why is this condition based on the formal types exactly matching rather than 
something about the FP semantics being different?  Formal types can correspond 
to the same format, right?

We need to check for saturation if we're either (1) decreasing the magnitude of 
the highest usable bit or (2) going signed->unsigned, (2) we're going 
signed->unsigned, or (3) we're going unsigned->signed without increasing the 
number of integral bits.  And I'd expect the checks we have to do in each case 
to be different.


================
Comment at: lib/Sema/SemaExpr.cpp:5889
+    case Type::STK_MemberPointer:
+      llvm_unreachable("Unimplemented conversion from FixedPoint to type");
+    }
----------------
Is there something I'm missing that actually diagnoses the unimplemented cases 
here?  There's a lot of code that seems to assume that any two arithmetic types 
can be converted to each other, and we do prefer not to crash the compiler, 
especially on valid code.


Repository:
  rC Clang

https://reviews.llvm.org/D50616



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

Reply via email to