sfertile added inline comments.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1633
+
+ if (BTy) {
+ BuiltinType::Kind BTyKind = BTy->getKind();
----------------
When can BTy be null? Should this instead be an assert?
Instead can we implement this without looking at the underlying type of the
bitfield?
Considering StorageUnitSize:
* for types smaller then unsigned we always increase the storage size to that
of unsigned.
* for int, and long in 32-bit the storage size is already 32 and we don't
adjust it.
* for long long in 32-bit we decrease the size to 32.
* For long long and long in 64-bit we leave the storage size as 64.
This could be implemented as
```
if (StorageUnitSize < Context.getTypeSize(Context.UnsignedIntTy) ||
(StorageUnitSize > Context.getTypeSize(Context.UnsignedIntTy) &&
Context.getTargetInfo().getTriple().isArch32Bit()))
StorageUnitSize = Context.getTypeSize(Context.UnsignedIntTy);
```
Without relying on extracting the underlying type and having to worry about the
case where BTy is null.
If we can also handle `FieldAlign `without knowing the underlying type then I
think we should do so and avoid trying to extract the underlying type
altogether.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1638
+ BTyKind != BuiltinType::LongLong) {
+ // Only set bitfields alignment to unsigned when it does
+ // not have attribute align specified or is less than
----------------
`to unsigned` --> `to alignof(unsigned)`
================
Comment at: clang/test/Layout/aix-bitfield-alignment.cpp:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -faix-pragma-pack -x c++ %s | \
----------------
I suggest we split the test cases which are C++ only into a sperate file, and
run the cases that are valid in both C and C++ as both to ensure there are no
differences between C and C++ layout fir bit fields.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87029/new/
https://reviews.llvm.org/D87029
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
