Author: yawanng Date: Tue Aug 1 14:41:39 2017 New Revision: 309750 URL: http://llvm.org/viewvc/llvm-project?rev=309750&view=rev Log: [clang] Change the condition of unnecessary packed warning
Summary: Change the condition of this unnecessary packed warning. The packed is unnecessary when 1. the alignment of the struct/class won't alter. 2. the size is unchanged. 3. the offset of each field is the same. Remove all field-level warning. Reviewers: chh, akyrtzi, rtrieu Reviewed By: chh Subscribers: rsmith, srhines, cfe-commits, xazax.hun Differential Revision: https://reviews.llvm.org/D34114 Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=309750&r1=309749&r2=309750&view=diff ============================================================================== --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original) +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Aug 1 14:41:39 2017 @@ -632,6 +632,9 @@ protected: /// pointer, as opposed to inheriting one from a primary base class. bool HasOwnVFPtr; + /// \brief the flag of field offset changing due to packed attribute. + bool HasPackedField; + typedef llvm::DenseMap<const CXXRecordDecl *, CharUnits> BaseOffsetsMapTy; /// Bases - base classes and their offsets in the record. @@ -666,7 +669,7 @@ protected: NonVirtualSize(CharUnits::Zero()), NonVirtualAlignment(CharUnits::One()), PrimaryBase(nullptr), PrimaryBaseIsVirtual(false), HasOwnVFPtr(false), - FirstNearlyEmptyVBase(nullptr) {} + HasPackedField(false), FirstNearlyEmptyVBase(nullptr) {} void Layout(const RecordDecl *D); void Layout(const CXXRecordDecl *D); @@ -1847,7 +1850,6 @@ void ItaniumRecordLayoutBuilder::FinishL uint64_t UnpaddedSize = getSizeInBits() - UnfilledBitsInLastUnit; uint64_t UnpackedSizeInBits = llvm::alignTo(getSizeInBits(), Context.toBits(UnpackedAlignment)); - CharUnits UnpackedSize = Context.toCharUnitsFromBits(UnpackedSizeInBits); uint64_t RoundedSize = llvm::alignTo(getSizeInBits(), Context.toBits(Alignment)); @@ -1882,10 +1884,11 @@ void ItaniumRecordLayoutBuilder::FinishL << (InBits ? 1 : 0); // (byte|bit) } - // Warn if we packed it unnecessarily. If the alignment is 1 byte don't - // bother since there won't be alignment issues. - if (Packed && UnpackedAlignment > CharUnits::One() && - getSize() == UnpackedSize) + // Warn if we packed it unnecessarily, when the unpacked alignment is not + // greater than the one after packing, the size in bits doesn't change and + // the offset of each field is identical. + if (Packed && UnpackedAlignment <= Alignment && + UnpackedSizeInBits == getSizeInBits() && !HasPackedField) Diag(D->getLocation(), diag::warn_unnecessary_packed) << Context.getTypeDeclType(RD); } @@ -1977,13 +1980,10 @@ void ItaniumRecordLayoutBuilder::CheckFi << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0); // (byte|bit) - } - - // Warn if we packed it unnecessarily. If the alignment is 1 byte don't - // bother since there won't be alignment issues. - if (isPacked && UnpackedAlign > CharBitNum && Offset == UnpackedOffset) - Diag(D->getLocation(), diag::warn_unnecessary_packed) - << D->getIdentifier(); + } + if (isPacked && Offset != UnpackedOffset) { + HasPackedField = true; + } } static const CXXMethodDecl *computeKeyFunction(ASTContext &Context, Modified: cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp?rev=309750&r1=309749&r2=309750&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp (original) +++ cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp Tue Aug 1 14:41:39 2017 @@ -17,7 +17,7 @@ struct S3 { } __attribute__((packed)); struct S4 { - int i; // expected-warning {{packed attribute is unnecessary for 'i'}} + int i; char c; } __attribute__((packed)); @@ -46,18 +46,18 @@ struct S8 : B { int i; // expected-warning {{padding struct 'S8' with 3 bytes to align 'i'}} }; -struct S9 { // expected-warning {{packed attribute is unnecessary for 'S9'}} - int x; // expected-warning {{packed attribute is unnecessary for 'x'}} - int y; // expected-warning {{packed attribute is unnecessary for 'y'}} +struct S9 { + int x; + int y; } __attribute__((packed)); -struct S10 { // expected-warning {{packed attribute is unnecessary for 'S10'}} - int x; // expected-warning {{packed attribute is unnecessary for 'x'}} +struct S10 { + int x; char a,b,c,d; } __attribute__((packed)); -struct S11 { +struct S11 { // expected-warning {{packed attribute is unnecessary for 'S11'}} bool x; char a,b,c,d; } __attribute__((packed)); @@ -72,5 +72,82 @@ struct S13 { // expected-warning {{paddi bool b : 10; }; +struct S14 { // expected-warning {{packed attribute is unnecessary for 'S14'}} + char a,b,c,d; +} __attribute__((packed)); + +struct S15 { // expected-warning {{packed attribute is unnecessary for 'S15'}} + struct S14 s; + char a; +} __attribute__((packed)); + +struct S16 { // expected-warning {{padding size of 'S16' with 2 bytes to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S16'}} + char a,b; +} __attribute__((packed, aligned(4))); + +struct S17 { + struct S16 s; + char a,b; +} __attribute__((packed, aligned(2))); + +struct S18 { // expected-warning {{padding size of 'S18' with 2 bytes to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S18'}} + struct S16 s; + char a,b; +} __attribute__((packed, aligned(4))); + +struct S19 { // expected-warning {{packed attribute is unnecessary for 'S19'}} + bool b; + char a; +} __attribute__((packed, aligned(1))); + +struct S20 { + int i; + char a; +} __attribute__((packed, aligned(1))); + +struct S21 { // expected-warning {{padding size of 'S21' with 4 bits to alignment boundary}} + unsigned char a : 6; + unsigned char b : 6; +} __attribute__((packed, aligned(1))); + +struct S22 { // expected-warning {{packed attribute is unnecessary for 'S22'}} + unsigned char a : 4; + unsigned char b : 4; +} __attribute__((packed)); + +struct S23 { // expected-warning {{padding size of 'S23' with 4 bits to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S23'}} + unsigned char a : 2; + unsigned char b : 2; +} __attribute__((packed)); + +struct S24 { + unsigned char a : 6; + unsigned char b : 6; + unsigned char c : 6; + unsigned char d : 6; + unsigned char e : 6; + unsigned char f : 6; + unsigned char g : 6; + unsigned char h : 6; +} __attribute__((packed)); + +struct S25 { // expected-warning {{padding size of 'S25' with 7 bits to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S25'}} + unsigned char a; + unsigned char b : 1; +} __attribute__((packed)); + +struct S26 { // expected-warning {{packed attribute is unnecessary for 'S26'}} + unsigned char a : 1; + unsigned char b; //expected-warning {{padding struct 'S26' with 7 bits to align 'b'}} +} __attribute__((packed)); + +struct S27 { // expected-warning {{padding size of 'S27' with 7 bits to alignment boundary}} + unsigned char a : 1; + unsigned char b : 8; +} __attribute__((packed)); + + // The warnings are emitted when the layout of the structs is computed, so we have to use them. -void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*) { } +void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*, + S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*, + S26*, S27*){} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits