llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Alex Bradbury (asb) <details> <summary>Changes</summary> Reverts llvm/llvm-project#<!-- -->130182 This is causing failures on RISC-V and ppc builders as mentioned on https://github.com/llvm/llvm-project/pull/130182#issuecomment-2775516899 Posting PR to check it reverts cleanly and pre-commit CI is happy. I'm happy with alternate fixes that don't require a revert, but getting this in the pipeline as one option. CC @<!-- -->therealcoochieman --- Full diff: https://github.com/llvm/llvm-project/pull/134239.diff 4 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (-2) - (modified) clang/lib/AST/RecordLayoutBuilder.cpp (+11-54) - (removed) clang/test/SemaCXX/windows-Wpadded-bitfield.cpp (-32) - (removed) clang/test/SemaCXX/windows-Wpadded.cpp (-40) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 47f9c3caa0e47..fdf9a246d6373 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -190,8 +190,6 @@ Modified Compiler Flags - The compiler flag `-fbracket-depth` default value is increased from 256 to 2048. (#GH94728) -- `-Wpadded` option implemented for the `x86_64-windows-msvc` target. Fixes #61702 - Removed Compiler Flags ------------------------- diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 41e7198cb7581..3e756ab9b9bfe 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2274,9 +2274,9 @@ static unsigned getPaddingDiagFromTagKind(TagTypeKind Tag) { } } -static void CheckFieldPadding(const ASTContext &Context, bool IsUnion, - uint64_t Offset, uint64_t UnpaddedOffset, - const FieldDecl *D) { +void ItaniumRecordLayoutBuilder::CheckFieldPadding( + uint64_t Offset, uint64_t UnpaddedOffset, uint64_t UnpackedOffset, + unsigned UnpackedAlign, bool isPacked, const FieldDecl *D) { // We let objc ivars without warning, objc interfaces generally are not used // for padding tricks. if (isa<ObjCIvarDecl>(D)) @@ -2300,8 +2300,7 @@ static void CheckFieldPadding(const ASTContext &Context, bool IsUnion, if (D->getIdentifier()) { auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_bitfield : diag::warn_padded_struct_field; - Context.getDiagnostics().Report(D->getLocation(), - Diagnostic) + Diag(D->getLocation(), Diagnostic) << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0) // (byte|bit) @@ -2309,22 +2308,15 @@ static void CheckFieldPadding(const ASTContext &Context, bool IsUnion, } else { auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_anon_bitfield : diag::warn_padded_struct_anon_field; - Context.getDiagnostics().Report(D->getLocation(), - Diagnostic) + Diag(D->getLocation(), Diagnostic) << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0); // (byte|bit) } - } -} - -void ItaniumRecordLayoutBuilder::CheckFieldPadding( - uint64_t Offset, uint64_t UnpaddedOffset, uint64_t UnpackedOffset, - unsigned UnpackedAlign, bool isPacked, const FieldDecl *D) { - ::CheckFieldPadding(Context, IsUnion, Offset, UnpaddedOffset, D); - if (isPacked && Offset != UnpackedOffset) { - HasPackedField = true; - } + } + if (isPacked && Offset != UnpackedOffset) { + HasPackedField = true; + } } static const CXXMethodDecl *computeKeyFunction(ASTContext &Context, @@ -2650,6 +2642,8 @@ struct MicrosoftRecordLayoutBuilder { /// virtual base classes and their offsets in the record. ASTRecordLayout::VBaseOffsetsMapTy VBases; /// The number of remaining bits in our last bitfield allocation. + /// This value isn't meaningful unless LastFieldIsNonZeroWidthBitfield is + /// true. unsigned RemainingBitsInField; bool IsUnion : 1; /// True if the last field laid out was a bitfield and was not 0 @@ -3010,15 +3004,6 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { } else { FieldOffset = Size.alignTo(Info.Alignment); } - - uint64_t UnpaddedFielddOffsetInBits = - Context.toBits(DataSize) - RemainingBitsInField; - - ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), - UnpaddedFielddOffsetInBits, FD); - - RemainingBitsInField = 0; - placeFieldAtOffset(FieldOffset); if (!IsOverlappingEmptyField) @@ -3064,14 +3049,10 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { } else { // Allocate a new block of memory and place the bitfield in it. CharUnits FieldOffset = Size.alignTo(Info.Alignment); - uint64_t UnpaddedFieldOffsetInBits = - Context.toBits(DataSize) - RemainingBitsInField; placeFieldAtOffset(FieldOffset); Size = FieldOffset + Info.Size; Alignment = std::max(Alignment, Info.Alignment); RemainingBitsInField = Context.toBits(Info.Size) - Width; - ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), - UnpaddedFieldOffsetInBits, FD); } DataSize = Size; } @@ -3095,14 +3076,9 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { } else { // Round up the current record size to the field's alignment boundary. CharUnits FieldOffset = Size.alignTo(Info.Alignment); - uint64_t UnpaddedFieldOffsetInBits = - Context.toBits(DataSize) - RemainingBitsInField; placeFieldAtOffset(FieldOffset); - RemainingBitsInField = 0; Size = FieldOffset; Alignment = std::max(Alignment, Info.Alignment); - ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), - UnpaddedFieldOffsetInBits, FD); } DataSize = Size; } @@ -3227,9 +3203,6 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { } void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { - uint64_t UnpaddedSizeInBits = Context.toBits(DataSize); - UnpaddedSizeInBits -= RemainingBitsInField; - // Respect required alignment. Note that in 32-bit mode Required alignment // may be 0 and cause size not to be updated. DataSize = Size; @@ -3258,22 +3231,6 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { Size = Context.toCharUnitsFromBits(External.Size); if (External.Align) Alignment = Context.toCharUnitsFromBits(External.Align); - return; - } - unsigned CharBitNum = Context.getTargetInfo().getCharWidth(); - uint64_t SizeInBits = Context.toBits(Size); - if (SizeInBits > UnpaddedSizeInBits) { - unsigned int PadSize = SizeInBits - UnpaddedSizeInBits; - bool InBits = true; - if (PadSize % CharBitNum == 0) { - PadSize = PadSize / CharBitNum; - InBits = false; - } - - Context.getDiagnostics().Report(RD->getLocation(), - diag::warn_padded_struct_size) - << Context.getTypeDeclType(RD) << PadSize - << (InBits ? 1 : 0); // (byte|bit) } } diff --git a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp deleted file mode 100644 index ee5a57124eca5..0000000000000 --- a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp +++ /dev/null @@ -1,32 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded %s - -struct __attribute__((ms_struct)) BitfieldStruct { // expected-warning {{padding size of 'BitfieldStruct' with 3 bytes to alignment boundary}} - char c : 1; - int : 0; // expected-warning {{padding struct 'BitfieldStruct' with 31 bits to align anonymous bit-field}} - char i; -}; - -struct __attribute__((ms_struct)) SevenBitfieldStruct { // expected-warning {{padding size of 'SevenBitfieldStruct' with 3 bytes to alignment boundary}} - char c : 7; - int : 0; // expected-warning {{padding struct 'SevenBitfieldStruct' with 25 bits to align anonymous bit-field}} - char i; -}; - -struct __attribute__((ms_struct)) SameUnitSizeBitfield { - char c : 7; - char : 1; // Same unit size attributes fall in the same unit + they fill the unit -> no padding - char i; -}; - -struct __attribute__((ms_struct)) DifferentUnitSizeBitfield { // expected-warning {{padding size of 'DifferentUnitSizeBitfield' with 3 bytes to alignment boundary}} - char c : 7; - int : 1; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' with 25 bits to align anonymous bit-field}} - char i; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' with 31 bits to align 'i'}} -}; - -int main() { - BitfieldStruct b; - SevenBitfieldStruct s; - SameUnitSizeBitfield su; - DifferentUnitSizeBitfield du; -} diff --git a/clang/test/SemaCXX/windows-Wpadded.cpp b/clang/test/SemaCXX/windows-Wpadded.cpp deleted file mode 100644 index da3f2bf08c6b8..0000000000000 --- a/clang/test/SemaCXX/windows-Wpadded.cpp +++ /dev/null @@ -1,40 +0,0 @@ -// RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -Wpadded %s - -struct __attribute__((ms_struct)) Foo { // expected-warning {{padding size of 'Foo' with 3 bytes to alignment boundary}} - int b : 1; - char a; // expected-warning {{padding struct 'Foo' with 31 bits to align 'a'}} -}; - -struct __attribute__((ms_struct)) AlignedStruct { // expected-warning {{padding size of 'AlignedStruct' with 4 bytes to alignment boundary}} - char c; - alignas(8) int i; // expected-warning {{padding struct 'AlignedStruct' with 7 bytes to align 'i'}} -}; - - -struct Base { - int b; -}; - -struct Derived : public Base { // expected-warning {{padding size of 'Derived' with 3 bytes to alignment boundary}} - char c; -}; - -union __attribute__((ms_struct)) Union { - char c; - long long u; -}; - -struct __attribute__((ms_struct)) StructWithUnion { // expected-warning {{padding size of 'StructWithUnion' with 6 bytes to alignment boundary}} - char c; - int : 0; - Union t; // expected-warning {{padding struct 'StructWithUnion' with 7 bytes to align 't'}} - short i; -}; - -int main() { - Foo f; - AlignedStruct a; - Derived d; - StructWithUnion swu; -} `````````` </details> https://github.com/llvm/llvm-project/pull/134239 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
