erik.pilkington added inline comments.

================
Comment at: clang/lib/AST/ExprConstant.cpp:6364-6371
+    const llvm::fltSemantics &Semantics = Info.Ctx.getFloatTypeSemantics(Ty);
+    unsigned NumBits = APFloat::semanticsSizeInBits(Semantics);
+    assert(NumBits % 8 == 0);
+    CharUnits Width = CharUnits::fromQuantity(NumBits / 8);
+    SmallVector<unsigned char, 8> Bytes(Width.getQuantity());
+    llvm::StoreIntToMemory(AsInt, &*Bytes.begin(), Width.getQuantity());
+    Buffer.writeObject(Offset, Bytes);
----------------
rsmith wrote:
> This appears more complex than necessary. `bitcastToAPInt` already returned 
> the correct number of bits (eg, 80 for x86 fp80) so you don't need to compute 
> that yourself. How about leaving this function alone and changing `visitInt` 
> to store the number of (value representation) bits in the `APSInt` rather 
> than the full (object representation) width of the type? As is, `visitInt` 
> would also be wrong in the same way if the integer type had any full bytes of 
> padding.
Sure, this causes us to have to handle `bool` in visitInt, but I think we 
should have been doing that anyways.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76323/new/

https://reviews.llvm.org/D76323



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

Reply via email to