dblaikie updated this revision to Diff 403825.
dblaikie marked 3 inline comments as done.
dblaikie added a comment.
rsmith's fixes for release notes
added some more test coverage to demonstrate that other fields are still packed
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117616/new/
https://reviews.llvm.org/D117616
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/LangOptions.h
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/SemaCXX/class-layout.cpp
Index: clang/test/SemaCXX/class-layout.cpp
===================================================================
--- clang/test/SemaCXX/class-layout.cpp
+++ clang/test/SemaCXX/class-layout.cpp
@@ -1,6 +1,9 @@
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
+// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=13
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=13 -DCLANG_ABI_COMPAT=13
// expected-no-diagnostics
#define SA(n, p) int a##n[(p) ? 1 : -1]
@@ -604,3 +607,37 @@
#endif
#pragma pack(pop)
}
+
+namespace non_pod {
+struct t1 {
+protected:
+ int a;
+};
+// GCC prints warning: ignoring packed attribute because of unpacked non-POD field 't1 t2::v1'`
+struct t2 {
+ char c1;
+ short s1;
+ char c2;
+ t1 v1;
+} __attribute__((packed));
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 13
+_Static_assert(_Alignof(t1) == 4, "");
+_Static_assert(_Alignof(t2) == 1, "");
+#else
+_Static_assert(_Alignof(t1) == 4, "");
+_Static_assert(_Alignof(t2) == 4, "");
+#endif
+_Static_assert(sizeof(t2) == 8, ""); // it's still packing the rest of the struct
+} // namespace non_pod
+
+namespace non_pod_packed {
+struct t1 {
+protected:
+ int a;
+} __attribute__((packed));
+struct t2 {
+ t1 v1;
+} __attribute__((packed));
+_Static_assert(_Alignof(t1) == 1, "");
+_Static_assert(_Alignof(t2) == 1, "");
+} // namespace non_pod_packed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3559,6 +3559,8 @@
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "11.0", SA);
else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver12)
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "12.0", SA);
+ else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver13)
+ GenerateArg(Args, OPT_fclang_abi_compat_EQ, "13.0", SA);
if (Opts.getSignReturnAddressScope() ==
LangOptions::SignReturnAddressScopeKind::All)
@@ -4061,6 +4063,8 @@
Opts.setClangABICompat(LangOptions::ClangABI::Ver11);
else if (Major <= 12)
Opts.setClangABICompat(LangOptions::ClangABI::Ver12);
+ else if (Major <= 13)
+ Opts.setClangABICompat(LangOptions::ClangABI::Ver13);
} else if (Ver != "latest") {
Diags.Report(diag::err_drv_invalid_value)
<< A->getAsString(Args) << A->getValue();
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1887,7 +1887,12 @@
UnfilledBitsInLastUnit = 0;
LastBitfieldStorageUnitSize = 0;
- bool FieldPacked = Packed || D->hasAttr<PackedAttr>();
+ llvm::Triple Target = Context.getTargetInfo().getTriple();
+ bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver13 ||
+ Target.isPS4() || Target.isOSDarwin())) ||
+ D->hasAttr<PackedAttr>();
AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
CharUnits FieldSize;
Index: clang/include/clang/Basic/LangOptions.h
===================================================================
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangOptions.h
@@ -181,6 +181,10 @@
/// global-scope inline variables incorrectly.
Ver12,
+ /// Attempt to be ABI-compatible with code generated by Clang 13.0.x.
+ /// This causes clang to not pack non-POD members of packed structs.
+ Ver13,
+
/// Conform to the underlying platform's C and C++ ABIs as closely
/// as we can.
Latest
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -236,6 +236,12 @@
is still in the process of being stabilized, so this type should not yet be
used in interfaces that require ABI stability.
+- GCC doesn't pack non-POD members in packed structs unless the packed
+ attribute is also specified on the member. Clang historically did perform
+ such packing. Clang now matches the gcc behavior (except on Darwin and PS4).
+ You can switch back to the old ABI behavior with the flag:
+ ``-fclang-abi-compat=13.0``.
+
OpenMP Support in Clang
-----------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
