[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith 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);
----------------
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.


================
Comment at: clang/lib/AST/ExprConstant.cpp:6462
+        APSInt Truncated = Val.trunc(IntWidth);
+        if (Truncated.zext(Val.getBitWidth()) != Val)
+          return unrepresentableValue(QualType(T, 0), Val);
----------------
If you want this to be a general check for integer types with padding, you should zext or sext as appropriate for the signedness of the truncated value. (Eg, you could set the signedness of `Truncated` to match the integer type, and use `extend` rather than `zext` here.)


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

https://reviews.llvm.org/D76323