Xiangling_L created this revision.
Xiangling_L added reviewers: jasonliu, hubert.reinterpretcast.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Xiangling_L requested review of this revision.
1.[bool, char, short] bitfields have the same alignment as unsigned int
2.Adjust alignment on typedef field decls/honor align attribute
3.Fix alignment for scoped enum class
4.Long long bitfield has 4bytes alignment and StorageUnitSize under 32 bit
compile mode
5.Emit error for oversized bitfield under 32bit mode
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D87029
Files:
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/Layout/aix-bitfield-alignment.cpp
clang/test/Layout/aix-oversized-bitfield.cpp
Index: clang/test/Layout/aix-oversized-bitfield.cpp
===================================================================
--- /dev/null
+++ clang/test/Layout/aix-oversized-bitfield.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fsyntax-only -verify %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only %s | \
+// RUN: FileCheck --check-prefix=CHECK64 %s
+
+struct A
+{
+ long long l : 64; // expected-error{{width of bit-field 'l' (64 bits) exceeds size of its type (32 bits)}}
+};
+
+int a = sizeof(A);
+
+// CHECK64: *** Dumping AST Record Layout
+// CHECK64-NEXT: 0 | struct A
+// CHECK64-NEXT: 0:0-63 | long long l
+// CHECK64-NEXT: | [sizeof=8, dsize=8, align=8, preferredalign=8,
+// CHECK64-NEXT: | nvsize=8, nvalign=8, preferrednvalign=8]
Index: clang/test/Layout/aix-bitfield-alignment.cpp
===================================================================
--- /dev/null
+++ clang/test/Layout/aix-bitfield-alignment.cpp
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -x c++ %s | \
+// RUN: FileCheck --check-prefixes=CHECK,CHECK32 %s
+
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -x c++ %s | \
+// RUN: FileCheck --check-prefixes=CHECK,CHECK64 %s
+
+struct A {
+ bool b : 3;
+ unsigned char c : 2;
+ unsigned short s : 6;
+};
+
+int a = sizeof(A);
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct A
+// CHECK-NEXT: 0:0-2 | _Bool b
+// CHECK-NEXT: 0:3-4 | unsigned char c
+// CHECK-NEXT: 0:5-10 | unsigned short s
+// CHECK-NEXT: | [sizeof=4, dsize=4, align=4, preferredalign=4,
+// CHECK-NEXT: | nvsize=4, nvalign=4, preferrednvalign=4]
+
+struct B {
+ char c;
+ int : 0;
+};
+
+int b = sizeof(B);
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct B
+// CHECK-NEXT: 0 | char c
+// CHECK-NEXT: 4:- | int
+// CHECK-NEXT: | [sizeof=4, dsize=4, align=4, preferredalign=4,
+// CHECK-NEXT: | nvsize=4, nvalign=4, preferrednvalign=4]
+
+struct C {
+ signed int a1 : 6;
+ signed char a2 : 4;
+ short int a3 : 2;
+ int a4 : 2;
+ signed long a5 : 5;
+ long long int a6 : 6;
+ unsigned long a7 : 8;
+};
+
+int c = sizeof(C);
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct C
+// CHECK-NEXT: 0:0-5 | int a1
+// CHECK-NEXT: 0:6-9 | signed char a2
+// CHECK-NEXT: 1:2-3 | short a3
+// CHECK-NEXT: 1:4-5 | int a4
+// CHECK-NEXT: 1:6-10 | long a5
+// CHECK-NEXT: 2:3-8 | long long a6
+// CHECK32: 4:0-7 | unsigned long a7
+// CHECK32: | [sizeof=8, dsize=8, align=4, preferredalign=4,
+// CHECK32: | nvsize=8, nvalign=4, preferrednvalign=4]
+// CHECK64: 3:1-8 | unsigned long a7
+// CHECK64: | [sizeof=8, dsize=8, align=8, preferredalign=8,
+// CHECK64: | nvsize=8, nvalign=8, preferrednvalign=8]
+
+typedef __attribute__((aligned(32))) short mySHORT;
+struct D {
+ char c : 8;
+ mySHORT : 0;
+};
+
+int d = sizeof(D);
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct D
+// CHECK-NEXT: 0:0-7 | char c
+// CHECK-NEXT: 32:- | mySHORT
+// CHECK-NEXT: | [sizeof=32, dsize=32, align=32, preferredalign=32,
+// CHECK-NEXT: | nvsize=32, nvalign=32, preferrednvalign=32]
+
+enum class Bool : bool { False = 0,
+ True = 1 };
+
+struct E {
+ Bool b : 1;
+};
+
+int e = sizeof(E);
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct E
+// CHECK-NEXT: 0:0-0 | enum Bool b
+// CHECK-NEXT: | [sizeof=4, dsize=4, align=4, preferredalign=4,
+// CHECK-NEXT: | nvsize=4, nvalign=4, preferrednvalign=4]
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16443,7 +16443,22 @@
bool MSBitfieldViolation =
Value.ugt(TypeStorageSize) &&
(IsMsStruct || Context.getTargetInfo().getCXXABI().isMicrosoft());
- if (CStdConstraintViolation || MSBitfieldViolation) {
+
+ bool AIXBitfieldViolation = false;
+ if (const BuiltinType *BTy = FieldTy.getTypePtr()->getAs<BuiltinType>()) {
+ if ((BTy->getKind() == BuiltinType::ULongLong ||
+ BTy->getKind() == BuiltinType::LongLong) &&
+ (unsigned)Value.getZExtValue() > 32 &&
+ Context.getTargetInfo().getTriple().isArch32Bit() &&
+ Context.getTargetInfo().getTriple().getOS() == llvm::Triple::AIX) {
+ AIXBitfieldViolation = true;
+ TypeStorageSize = 32;
+ TypeWidth = 32;
+ }
+ }
+
+ if (CStdConstraintViolation || MSBitfieldViolation ||
+ AIXBitfieldViolation) {
unsigned DiagWidth =
CStdConstraintViolation ? TypeWidth : TypeStorageSize;
if (FieldName)
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -622,9 +622,9 @@
/// an adjacent bitfield if necessary. The unit in question is usually
/// a byte, but larger units are used if IsMsStruct.
unsigned char UnfilledBitsInLastUnit;
- /// LastBitfieldTypeSize - If IsMsStruct, represents the size of the type
- /// of the previous field if it was a bitfield.
- unsigned char LastBitfieldTypeSize;
+ /// LastBitfieldStorageUnitSize - If IsMsStruct, represents the size of the
+ /// storage unit of the previous field if it was a bitfield.
+ unsigned char LastBitfieldStorageUnitSize;
/// MaxFieldAlignment - The maximum allowed field alignment. This is set by
/// #pragma pack.
@@ -693,7 +693,7 @@
UnadjustedAlignment(CharUnits::One()), UseExternalLayout(false),
InferAlignment(false), Packed(false), IsUnion(false),
IsMac68kAlign(false), IsMsStruct(false), UnfilledBitsInLastUnit(0),
- LastBitfieldTypeSize(0), MaxFieldAlignment(CharUnits::Zero()),
+ LastBitfieldStorageUnitSize(0), MaxFieldAlignment(CharUnits::Zero()),
DataSize(0), NonVirtualSize(CharUnits::Zero()),
NonVirtualAlignment(CharUnits::One()),
PreferredNVAlignment(CharUnits::One()),
@@ -708,7 +708,7 @@
void LayoutFields(const RecordDecl *D);
void LayoutField(const FieldDecl *D, bool InsertExtraPadding);
- void LayoutWideBitField(uint64_t FieldSize, uint64_t TypeSize,
+ void LayoutWideBitField(uint64_t FieldSize, uint64_t StorageUnitSize,
bool FieldPacked, const FieldDecl *D);
void LayoutBitField(const FieldDecl *D);
@@ -1451,7 +1451,7 @@
}
void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
- uint64_t TypeSize,
+ uint64_t StorageUnitSize,
bool FieldPacked,
const FieldDecl *D) {
assert(Context.getLangOpts().CPlusPlus &&
@@ -1481,7 +1481,7 @@
// We're not going to use any of the unfilled bits in the last byte.
UnfilledBitsInLastUnit = 0;
- LastBitfieldTypeSize = 0;
+ LastBitfieldStorageUnitSize = 0;
uint64_t FieldOffset;
uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit;
@@ -1516,11 +1516,15 @@
UpdateAlignment(TypeAlign);
}
+static bool isAIXLayout(const ASTContext &Context) {
+ return Context.getTargetInfo().getTriple().getOS() == llvm::Triple::AIX;
+}
+
void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
bool FieldPacked = Packed || D->hasAttr<PackedAttr>();
uint64_t FieldSize = D->getBitWidthValue(Context);
TypeInfo FieldInfo = Context.getTypeInfo(D->getType());
- uint64_t TypeSize = FieldInfo.Width;
+ uint64_t StorageUnitSize = FieldInfo.Width;
unsigned FieldAlign = FieldInfo.Align;
// UnfilledBitsInLastUnit is the difference between the end of the
@@ -1529,7 +1533,7 @@
// 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).
+ // LastBitfieldStorageUnitSize (if set).
// The struct-layout algorithm is dictated by the platform ABI,
// which in principle could use almost any rules it likes. In
@@ -1583,26 +1587,64 @@
// First, some simple bookkeeping to perform for ms_struct structs.
if (IsMsStruct) {
// The field alignment for integer types is always the size.
- FieldAlign = TypeSize;
+ FieldAlign = StorageUnitSize;
// If the previous field was not a bitfield, or was a bitfield
// with a different storage unit size, or if this field doesn't fit into
// the current storage unit, we're done with that storage unit.
- if (LastBitfieldTypeSize != TypeSize ||
+ if (LastBitfieldStorageUnitSize != StorageUnitSize ||
UnfilledBitsInLastUnit < FieldSize) {
// Also, ignore zero-length bitfields after non-bitfields.
- if (!LastBitfieldTypeSize && !FieldSize)
+ if (!LastBitfieldStorageUnitSize && !FieldSize)
FieldAlign = 1;
UnfilledBitsInLastUnit = 0;
- LastBitfieldTypeSize = 0;
+ LastBitfieldStorageUnitSize = 0;
+ }
+ }
+
+ // On AIX, [bool, char, short] bitfields have the same alignment
+ // as [unsigned].
+ if (isAIXLayout(Context)) {
+ const BuiltinType *BTy = nullptr;
+
+ // Get the underlying type from EnumDecl.
+ if (const EnumType *ET = dyn_cast<EnumType>(D->getType().getTypePtr())) {
+ const EnumDecl *ED = ET->getDecl();
+ if (const BuiltinType *BT =
+ dyn_cast<BuiltinType>(ED->getPromotionType().getTypePtr()))
+ BTy = BT;
+ } else {
+ BTy = Context.getBaseElementType(D->getType())->getAs<BuiltinType>();
+ }
+
+ if (BTy) {
+ BuiltinType::Kind BTyKind = BTy->getKind();
+ if (BTyKind != BuiltinType::ULong && BTyKind != BuiltinType::Long &&
+ BTyKind != BuiltinType::ULongLong &&
+ BTyKind != BuiltinType::LongLong) {
+ // Only set bitfields alignment to unsigned when it does
+ // not have attribute align specified or is less than
+ // unsigned alignment.
+ if (FieldAlign < Context.getTypeSize(Context.UnsignedIntTy))
+ FieldAlign = Context.getTypeSize(Context.UnsignedIntTy);
+ StorageUnitSize = Context.getTypeSize(Context.UnsignedIntTy);
+ } else {
+ // Handle AIX oversized Long Long bitfield under 32 bit compile mode.
+ if (StorageUnitSize > 32 &&
+ Context.getTargetInfo().getTriple().isArch32Bit()) {
+ StorageUnitSize = 32;
+ FieldAlign = 32;
+ }
+ }
}
}
// If the field is wider than its declared type, it follows
- // different rules in all cases.
- if (FieldSize > TypeSize) {
- LayoutWideBitField(FieldSize, TypeSize, FieldPacked, D);
+ // different rules in all cases, except on AIX.
+ // On AIX, wide bitfield follows the same rules as normal bitfield.
+ if (FieldSize > StorageUnitSize) {
+ LayoutWideBitField(FieldSize, StorageUnitSize, FieldPacked, D);
return;
}
@@ -1686,7 +1728,7 @@
// Compute the real offset.
if (FieldSize == 0 ||
(AllowPadding &&
- (FieldOffset & (FieldAlign-1)) + FieldSize > TypeSize)) {
+ (FieldOffset & (FieldAlign - 1)) + FieldSize > StorageUnitSize)) {
FieldOffset = llvm::alignTo(FieldOffset, FieldAlign);
} else if (ExplicitFieldAlign &&
(MaxFieldAlignmentInBits == 0 ||
@@ -1700,7 +1742,8 @@
// Repeat the computation for diagnostic purposes.
if (FieldSize == 0 ||
(AllowPadding &&
- (UnpackedFieldOffset & (UnpackedFieldAlign-1)) + FieldSize > TypeSize))
+ (UnpackedFieldOffset & (UnpackedFieldAlign - 1)) + FieldSize >
+ StorageUnitSize))
UnpackedFieldOffset =
llvm::alignTo(UnpackedFieldOffset, UnpackedFieldAlign);
else if (ExplicitFieldAlign &&
@@ -1741,11 +1784,11 @@
// is a zero-width bitfield, in which case just use a size of 1.
uint64_t RoundedFieldSize;
if (IsMsStruct) {
- RoundedFieldSize =
- (FieldSize ? TypeSize : Context.getTargetInfo().getCharWidth());
+ RoundedFieldSize = (FieldSize ? StorageUnitSize
+ : Context.getTargetInfo().getCharWidth());
- // Otherwise, allocate just the number of bytes required to store
- // the bitfield.
+ // Otherwise, allocate just the number of bytes required to store
+ // the bitfield.
} else {
RoundedFieldSize = roundUpSizeToCharAlignment(FieldSize, Context);
}
@@ -1757,15 +1800,15 @@
// We should have cleared UnfilledBitsInLastUnit in every case
// where we changed storage units.
if (!UnfilledBitsInLastUnit) {
- setDataSize(FieldOffset + TypeSize);
- UnfilledBitsInLastUnit = TypeSize;
+ setDataSize(FieldOffset + StorageUnitSize);
+ UnfilledBitsInLastUnit = StorageUnitSize;
}
UnfilledBitsInLastUnit -= FieldSize;
- LastBitfieldTypeSize = TypeSize;
+ LastBitfieldStorageUnitSize = StorageUnitSize;
- // 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.
+ // 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();
@@ -1775,7 +1818,7 @@
// 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;
+ LastBitfieldStorageUnitSize = 0;
}
// Update the size.
@@ -1825,7 +1868,7 @@
uint64_t UnpaddedFieldOffset = getDataSizeInBits() - UnfilledBitsInLastUnit;
// Reset the unfilled bits.
UnfilledBitsInLastUnit = 0;
- LastBitfieldTypeSize = 0;
+ LastBitfieldStorageUnitSize = 0;
bool FieldPacked = Packed || D->hasAttr<PackedAttr>();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
