yawanng updated this revision to Diff 108395.
yawanng edited the summary of this revision.
yawanng added a comment.
Including cases '__attribute__((packed, aligned(X)));'. When the alignment
before packing is less or equal to the packed one, he 'packed' attribute not
including 'aligned(X)' seems unnecessary. Because the alignment won't change
as well as the size after removing 'packed'.
https://reviews.llvm.org/D34114
Files:
lib/AST/RecordLayoutBuilder.cpp
test/CodeGenCXX/warn-padded-packed.cpp
Index: test/CodeGenCXX/warn-padded-packed.cpp
===================================================================
--- test/CodeGenCXX/warn-padded-packed.cpp
+++ test/CodeGenCXX/warn-padded-packed.cpp
@@ -17,7 +17,7 @@
} __attribute__((packed));
struct S4 {
- int i; // expected-warning {{packed attribute is unnecessary for 'i'}}
+ int i;
char c;
} __attribute__((packed));
@@ -46,18 +46,18 @@
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,28 @@
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 boundar}} 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 boundar}} expected-warning {{packed attribute is unnecessary for
'S18'}}
+ struct S16 s;
+ char a,b;
+} __attribute__((packed, aligned(4)));
+
// 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*) { }
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -1882,10 +1882,9 @@
<< (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.
+ if (Packed && UnpackedAlignment <= Alignment || )
Diag(D->getLocation(), diag::warn_unnecessary_packed)
<< Context.getTypeDeclType(RD);
}
@@ -1978,12 +1977,6 @@
<< 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();
}
static const CXXMethodDecl *computeKeyFunction(ASTContext &Context,
Index: test/CodeGenCXX/warn-padded-packed.cpp
===================================================================
--- test/CodeGenCXX/warn-padded-packed.cpp
+++ test/CodeGenCXX/warn-padded-packed.cpp
@@ -17,7 +17,7 @@
} __attribute__((packed));
struct S4 {
- int i; // expected-warning {{packed attribute is unnecessary for 'i'}}
+ int i;
char c;
} __attribute__((packed));
@@ -46,18 +46,18 @@
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,28 @@
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 boundar}} 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 boundar}} expected-warning {{packed attribute is unnecessary for 'S18'}}
+ struct S16 s;
+ char a,b;
+} __attribute__((packed, aligned(4)));
+
// 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*) { }
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -1882,10 +1882,9 @@
<< (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.
+ if (Packed && UnpackedAlignment <= Alignment || )
Diag(D->getLocation(), diag::warn_unnecessary_packed)
<< Context.getTypeDeclType(RD);
}
@@ -1978,12 +1977,6 @@
<< 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();
}
static const CXXMethodDecl *computeKeyFunction(ASTContext &Context,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
