From 893b864cb855e624c0f72da4b66e758e1f073f20 Mon Sep 17 00:00:00 2001 From: Tom Stellard Date: Thu, 10 Mar 2022 12:52:12 -0800 Subject: Revert "GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs" This reverts commit 277123376ce08c98b07c154bf83e4092a5d4d3c6. There are still some issues with this change to be worked out, so revert this patch to avoid having to change ABIs from LLVM 13->14 and then again from LLVM 14->15. See discussion in https://reviews.llvm.org/D118511 --- clang/docs/ReleaseNotes.rst | 6 ----- clang/include/clang/Basic/LangOptions.h | 4 ---- clang/lib/AST/RecordLayoutBuilder.cpp | 7 +----- clang/lib/Frontend/CompilerInvocation.cpp | 4 ---- clang/test/SemaCXX/class-layout.cpp | 37 ------------------------------- 5 files changed, 1 insertion(+), 57 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 4eaa589b542d..5821e41fc733 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -338,12 +338,6 @@ ABI Changes in Clang 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 ----------------------- diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 50c7f038fc6b..09afa641acf9 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -181,10 +181,6 @@ public: /// 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 diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 709e05716a56..61a30ead165e 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -1887,12 +1887,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, UnfilledBitsInLastUnit = 0; LastBitfieldStorageUnitSize = 0; - llvm::Triple Target = Context.getTargetInfo().getTriple(); - bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() || - Context.getLangOpts().getClangABICompat() <= - LangOptions::ClangABI::Ver13 || - Target.isPS4() || Target.isOSDarwin())) || - D->hasAttr(); + bool FieldPacked = Packed || D->hasAttr(); AlignRequirementKind AlignRequirement = AlignRequirementKind::None; CharUnits FieldSize; diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 553a0b31c0ab..7f1ce3da7e7e 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -3560,8 +3560,6 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts, 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) @@ -4064,8 +4062,6 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args, 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(); diff --git a/clang/test/SemaCXX/class-layout.cpp b/clang/test/SemaCXX/class-layout.cpp index 79fa67707110..5403bd6e6a6f 100644 --- a/clang/test/SemaCXX/class-layout.cpp +++ b/clang/test/SemaCXX/class-layout.cpp @@ -1,9 +1,6 @@ // 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] @@ -607,37 +604,3 @@ namespace PR37275 { #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 -- cgit v1.2.3