[libc-commits] [PATCH] D105561: [libc] Capture floating point encoding and arrange it sequentially in memory

[Adrian McCarthy via Phabricator via libc-commits]

amccarth added inline comments.


================
Comment at: libc/utils/FPUtil/FPBits.h:49
+
+  UIntType bits;
+
----------------
I'm rather surprised that this change was necessary.  When working with a specific bit layout, using a unsigned integer type and bitwise operations can be a good idea--I don't think the language standards give many guarantees about how the bits are allocated (e.g., MSB first or LSB first?).  But I'm not sure why the compiler (we are talking about clang here, right?) didn't pack the structures the same way on all platforms.

Perhaps when targeting Windows, clang is trying too hard to be compatible with MSVC.  With MSVC, when defining bit fields, they will pack by default--but only if the underlying types are all the same.  Since the old types were UIntType,, uint16_t, and uint8_t--all of which have different alignment requirements--MSVC doesn't pack those.  But if they had all been defined as uint32_t, MSVC would certainly have done the right thing.

And thus, maybe clang would have as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105561