r200379 - Extensively comment bitfield layout, rearrange some
John McCall
rjmccall at apple.com
Tue Jan 28 23:53:44 PST 2014
Author: rjmccall
Date: Wed Jan 29 01:53:44 2014
New Revision: 200379
URL: http://llvm.org/viewvc/llvm-project?rev=200379&view=rev
Log:
Extensively comment bitfield layout, rearrange some
code for legibility, and fix a bug with bitfields in packed
ms_structs.
rdar://15926990
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/test/CodeGen/ms_struct-pack.c
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=200379&r1=200378&r2=200379&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Wed Jan 29 01:53:44 2014
@@ -1450,127 +1450,191 @@ void RecordLayoutBuilder::LayoutBitField
uint64_t TypeSize = FieldInfo.first;
unsigned FieldAlign = FieldInfo.second;
+ // UnfilledBitsInLastUnit is the difference between the end of the
+ // last allocated bitfield (i.e. the first bit offset available for
+ // bitfields) and the end of the current data size in bits (i.e. the
+ // first bit offset available for non-bitfields). The current data
+ // size in bits is always a multiple of the char size; additionally,
+ // for ms_struct records it's also a multiple of the
+ // LastBitfieldTypeSize (if set).
+
+ // The basic bitfield layout rule for ms_struct is to allocate an
+ // entire unit of the bitfield's declared type (e.g. 'unsigned
+ // long'), then parcel it up among successive bitfields whose
+ // declared types have the same size, making a new unit as soon as
+ // the last can no longer store the whole value.
+
+ // The standard bitfield layout rule for non-ms_struct is to place
+ // bitfields at the next available bit offset where the entire
+ // bitfield would fit in an aligned storage unit of the declared
+ // type (even if there are also non-bitfields within that same
+ // unit). However, some targets (those that !useBitFieldTypeAlignment())
+ // don't require this storage unit to be aligned, and therefore
+ // always put the bit-field at the next available bit offset.
+ // Such targets generally do interpret zero-width bitfields as
+ // forcing the use of a new storage unit.
+
+ // First, some simple bookkeeping to perform for ms_struct structs.
if (IsMsStruct) {
- // The field alignment for integer types in ms_struct structs is
- // always the size.
+ // The field alignment for integer types is always the size.
FieldAlign = TypeSize;
- // Ignore zero-length bitfields after non-bitfields in ms_struct structs.
- if (!FieldSize && !LastBitfieldTypeSize)
- FieldAlign = 1;
- // If a bitfield is followed by a bitfield of a different size, don't
- // pack the bits together in ms_struct structs.
+
+ // If the previous field was not a bitfield, or was a bitfield
+ // with a different storage unit size, we're done with that
+ // storage unit.
if (LastBitfieldTypeSize != TypeSize) {
+ // Also, ignore zero-length bitfields after non-bitfields.
+ if (!LastBitfieldTypeSize && !FieldSize)
+ FieldAlign = 1;
+
UnfilledBitsInLastUnit = 0;
LastBitfieldTypeSize = 0;
}
}
- uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit;
- uint64_t FieldOffset = IsUnion ? 0 : UnpaddedFieldOffset;
-
- bool ZeroLengthBitfield = false;
- if (!Context.getTargetInfo().useBitFieldTypeAlignment() &&
- Context.getTargetInfo().useZeroLengthBitfieldAlignment() &&
- FieldSize == 0) {
- // The alignment of a zero-length bitfield affects the alignment
- // of the next member. The alignment is the max of the zero
- // length bitfield's alignment and a target specific fixed value.
- ZeroLengthBitfield = true;
- unsigned ZeroLengthBitfieldBoundary =
- Context.getTargetInfo().getZeroLengthBitfieldBoundary();
- if (ZeroLengthBitfieldBoundary > FieldAlign)
- FieldAlign = ZeroLengthBitfieldBoundary;
- }
-
+ // If the field is wider than its declared type, it follows
+ // different rules in all cases.
if (FieldSize > TypeSize) {
LayoutWideBitField(FieldSize, TypeSize, FieldPacked, D);
return;
}
- // The align if the field is not packed. This is to check if the attribute
- // was unnecessary (-Wpacked).
+ // Compute the next available bit offset.
+ uint64_t FieldOffset =
+ IsUnion ? 0 : (getDataSizeInBits() - UnfilledBitsInLastUnit);
+
+ // Handle targets that don't honor bitfield type alignment.
+ if (!Context.getTargetInfo().useBitFieldTypeAlignment()) {
+ // Some such targets do honor it on zero-width bitfields.
+ if (FieldSize == 0 &&
+ Context.getTargetInfo().useZeroLengthBitfieldAlignment()) {
+ // The alignment to round up to is the max of the field's natural
+ // alignment and a target-specific fixed value (sometimes zero).
+ unsigned ZeroLengthBitfieldBoundary =
+ Context.getTargetInfo().getZeroLengthBitfieldBoundary();
+ FieldAlign = std::max(FieldAlign, ZeroLengthBitfieldBoundary);
+
+ // If that doesn't apply, just ignore the field alignment.
+ } else {
+ FieldAlign = 1;
+ }
+ }
+
+ // Remember the alignment we would have used if the field were not packed.
unsigned UnpackedFieldAlign = FieldAlign;
- uint64_t UnpackedFieldOffset = FieldOffset;
- if (!Context.getTargetInfo().useBitFieldTypeAlignment() && !ZeroLengthBitfield)
- UnpackedFieldAlign = 1;
- if (FieldPacked ||
- (!Context.getTargetInfo().useBitFieldTypeAlignment() && !ZeroLengthBitfield))
+ // Ignore the field alignment if the field is packed.
+ if (FieldPacked)
FieldAlign = 1;
- FieldAlign = std::max(FieldAlign, D->getMaxAlignment());
- UnpackedFieldAlign = std::max(UnpackedFieldAlign, D->getMaxAlignment());
- // The maximum field alignment overrides the aligned attribute.
- if (!MaxFieldAlignment.isZero() && FieldSize != 0) {
+ // But, if there's an 'aligned' attribute on the field, honor that.
+ if (unsigned ExplicitFieldAlign = D->getMaxAlignment()) {
+ FieldAlign = std::max(FieldAlign, ExplicitFieldAlign);
+ UnpackedFieldAlign = std::max(UnpackedFieldAlign, ExplicitFieldAlign);
+ }
+
+ // But, if there's a #pragma pack in play, that takes precedent over
+ // even the 'aligned' attribute, for non-zero-width bitfields.
+ if (!MaxFieldAlignment.isZero() && FieldSize) {
unsigned MaxFieldAlignmentInBits = Context.toBits(MaxFieldAlignment);
FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
}
- // ms_struct bitfields always have to start at a round alignment.
- if (IsMsStruct && !LastBitfieldTypeSize) {
- FieldOffset = llvm::RoundUpToAlignment(FieldOffset, FieldAlign);
- UnpackedFieldOffset = llvm::RoundUpToAlignment(UnpackedFieldOffset,
- UnpackedFieldAlign);
- }
-
- // Check if we need to add padding to give the field the correct alignment.
- if (FieldSize == 0 ||
- (MaxFieldAlignment.isZero() &&
- (FieldOffset & (FieldAlign-1)) + FieldSize > TypeSize))
- FieldOffset = llvm::RoundUpToAlignment(FieldOffset, FieldAlign);
-
- if (FieldSize == 0 ||
- (MaxFieldAlignment.isZero() &&
- (UnpackedFieldOffset & (UnpackedFieldAlign-1)) + FieldSize > TypeSize))
- UnpackedFieldOffset = llvm::RoundUpToAlignment(UnpackedFieldOffset,
- UnpackedFieldAlign);
-
- // Padding members don't affect overall alignment, unless zero length bitfield
- // alignment is enabled.
- if (!D->getIdentifier() &&
- !Context.getTargetInfo().useZeroLengthBitfieldAlignment() &&
- !IsMsStruct)
- FieldAlign = UnpackedFieldAlign = 1;
+ // For purposes of diagnostics, we're going to simultaneously
+ // compute the field offsets that we would have used if we weren't
+ // adding any alignment padding or if the field weren't packed.
+ uint64_t UnpaddedFieldOffset = FieldOffset;
+ uint64_t UnpackedFieldOffset = FieldOffset;
+
+ // Check if we need to add padding to fit the bitfield within an
+ // allocation unit with the right size and alignment. The rules are
+ // somewhat different here for ms_struct structs.
+ if (IsMsStruct) {
+ // If it's not a zero-width bitfield, and we can fit the bitfield
+ // into the active storage unit (and we haven't already decided to
+ // start a new storage unit), just do so, regardless of any other
+ // other consideration. Otherwise, round up to the right alignment.
+ if (FieldSize == 0 || FieldSize > UnfilledBitsInLastUnit) {
+ FieldOffset = llvm::RoundUpToAlignment(FieldOffset, FieldAlign);
+ UnpackedFieldOffset = llvm::RoundUpToAlignment(UnpackedFieldOffset,
+ UnpackedFieldAlign);
+ UnfilledBitsInLastUnit = 0;
+ }
+
+ } else {
+ // #pragma pack, with any value, suppresses the insertion of padding.
+ bool AllowPadding = MaxFieldAlignment.isZero();
+ // Compute the real offset.
+ if (FieldSize == 0 ||
+ (AllowPadding &&
+ (FieldOffset & (FieldAlign-1)) + FieldSize > TypeSize)) {
+ FieldOffset = llvm::RoundUpToAlignment(FieldOffset, FieldAlign);
+ }
+
+ // Repeat the computation for diagnostic purposes.
+ if (FieldSize == 0 ||
+ (AllowPadding &&
+ (UnpackedFieldOffset & (UnpackedFieldAlign-1)) + FieldSize > TypeSize))
+ UnpackedFieldOffset = llvm::RoundUpToAlignment(UnpackedFieldOffset,
+ UnpackedFieldAlign);
+ }
+
+ // If we're using external layout, give the external layout a chance
+ // to override this information.
if (ExternalLayout)
FieldOffset = updateExternalFieldOffset(D, FieldOffset);
- // Place this field at the current location.
+ // Okay, place the bitfield at the calculated offset.
FieldOffsets.push_back(FieldOffset);
+ // Bookkeeping:
+
+ // Anonymous members don't affect the overall record alignment,
+ // except on targets where they do.
+ if (!IsMsStruct &&
+ !Context.getTargetInfo().useZeroLengthBitfieldAlignment() &&
+ !D->getIdentifier())
+ FieldAlign = UnpackedFieldAlign = 1;
+
+ // Diagnose differences in layout due to padding or packing.
if (!ExternalLayout)
CheckFieldPadding(FieldOffset, UnpaddedFieldOffset, UnpackedFieldOffset,
UnpackedFieldAlign, FieldPacked, D);
// Update DataSize to include the last byte containing (part of) the bitfield.
+
+ // For unions, this is just a max operation, as usual.
if (IsUnion) {
// FIXME: I think FieldSize should be TypeSize here.
setDataSize(std::max(getDataSizeInBits(), FieldSize));
- } else {
- if (IsMsStruct && FieldSize) {
- // Under ms_struct, a bitfield always takes up space equal to the size
- // of the type. We can't just change the alignment computation on the
- // other codepath because of the way this interacts with #pragma pack:
- // in a packed struct, we need to allocate misaligned space in the
- // struct to hold the bitfield.
- if (!UnfilledBitsInLastUnit) {
- setDataSize(FieldOffset + TypeSize);
- UnfilledBitsInLastUnit = TypeSize - FieldSize;
- } else if (UnfilledBitsInLastUnit < FieldSize) {
- setDataSize(getDataSizeInBits() + TypeSize);
- UnfilledBitsInLastUnit = TypeSize - FieldSize;
- } else {
- UnfilledBitsInLastUnit -= FieldSize;
- }
- LastBitfieldTypeSize = TypeSize;
- } else {
- uint64_t NewSizeInBits = FieldOffset + FieldSize;
- uint64_t BitfieldAlignment = Context.getTargetInfo().getCharAlign();
- setDataSize(llvm::RoundUpToAlignment(NewSizeInBits, BitfieldAlignment));
- UnfilledBitsInLastUnit = getDataSizeInBits() - NewSizeInBits;
- LastBitfieldTypeSize = 0;
+
+ // For non-zero-width bitfields in ms_struct structs, allocate a new
+ // storage unit if necessary.
+ } else if (IsMsStruct && FieldSize) {
+ // We should have cleared UnfilledBitsInLastUnit in every case
+ // where we changed storage units.
+ if (!UnfilledBitsInLastUnit) {
+ setDataSize(FieldOffset + TypeSize);
+ UnfilledBitsInLastUnit = TypeSize;
}
+ UnfilledBitsInLastUnit -= FieldSize;
+ LastBitfieldTypeSize = TypeSize;
+
+ // Otherwise, bump the data size up to include the bitfield,
+ // including padding up to char alignment, and then remember how
+ // bits we didn't use.
+ } else {
+ uint64_t NewSizeInBits = FieldOffset + FieldSize;
+ uint64_t CharAlignment = Context.getTargetInfo().getCharAlign();
+ setDataSize(llvm::RoundUpToAlignment(NewSizeInBits, CharAlignment));
+ UnfilledBitsInLastUnit = getDataSizeInBits() - NewSizeInBits;
+
+ // The only time we can get here for an ms_struct is if this is a
+ // zero-width bitfield, which doesn't count as anything for the
+ // purposes of unfilled bits.
+ LastBitfieldTypeSize = 0;
}
// Update the size.
Modified: cfe/trunk/test/CodeGen/ms_struct-pack.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms_struct-pack.c?rev=200379&r1=200378&r2=200379&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/ms_struct-pack.c (original)
+++ cfe/trunk/test/CodeGen/ms_struct-pack.c Wed Jan 29 01:53:44 2014
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm-only -triple i386-apple-darwin9 %s
+// RUN: %clang_cc1 -emit-llvm-only -triple i386-apple-darwin9 -fdump-record-layouts %s | FileCheck %s
// rdar://8823265
#pragma pack(1)
@@ -123,3 +123,22 @@ typedef struct _eight_ms eight_ms;
static int a8[(sizeof(eight_ms) == 48) - 1];
+// rdar://15926990
+#pragma pack(2)
+struct test0 {
+ unsigned long a : 8;
+ unsigned long b : 8;
+ unsigned long c : 8;
+ unsigned long d : 10;
+ unsigned long e : 1;
+} __attribute__((__ms_struct__));
+
+// CHECK: Type: struct test0
+// CHECK-NEXT: Record:
+// CHECK-NEXT: Layout:
+// CHECK-NEXT: Size:64
+// CHECK-NEXT: DataSize:64
+// CHECK-NEXT: Alignment:16
+// CHECK-NEXT: FieldOffsets: [0, 8, 16, 32, 42]>
+
+static int test0[(sizeof(struct test0) == 8) ? 1 : -1];
More information about the cfe-commits
mailing list