llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-x86 Author: Phoebe Wang (phoebewang) <details> <summary>Changes</summary> Follow up of #<!-- -->216777. Assisted-by: Claude Opus 4.8 --- Full diff: https://github.com/llvm/llvm-project/pull/220788.diff 7 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+2-1) - (modified) clang/include/clang/Basic/ABIVersions.def (+6) - (modified) clang/lib/CodeGen/CodeGenModule.cpp (+1) - (modified) clang/lib/CodeGen/Targets/X86.cpp (+12-4) - (added) clang/test/CodeGen/X86/x86_64-unnamed-bitfield-abi-compat.c (+30) - (modified) llvm/include/llvm/ABI/TargetInfo.h (+2-1) - (modified) llvm/lib/ABI/Targets/X86.cpp (+7-3) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index bf295981710ac..17c10fd4ff86f 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -106,7 +106,8 @@ features cannot lower the translation-unit ABI level; Aggregates where this changes the classification may be passed or returned differently -- a struct holding a run of `__int128` bit-fields, for example, now travels in the two integer registers the ABI assigns it. This also fixes - a crash when such a struct was passed or returned. (#GH202205) + a crash when such a struct was passed or returned. `-fclang-abi-compat=23` + restores the previous behavior. (#GH202205) ### AST Dumping Potentially Breaking Changes diff --git a/clang/include/clang/Basic/ABIVersions.def b/clang/include/clang/Basic/ABIVersions.def index e1017a1547773..3c434da91bfab 100644 --- a/clang/include/clang/Basic/ABIVersions.def +++ b/clang/include/clang/Basic/ABIVersions.def @@ -161,6 +161,12 @@ ABI_VER_MAJOR(22) /// - On MIPS N32/N64, always pass a `_Complex float` or `_Complex double` /// argument as its two parts, one floating-point register each, instead of /// packing it into integer registers once there is no room for both. +/// - On x86-64 System V, skip every unnamed bit-field as padding when +/// classifying an aggregate, instead of treating a non-zero-width unnamed +/// bit-field as INTEGER storage like a named one the way GCC does. (This +/// faithfully reproduces Clang 23, including its crash on aggregates such +/// as a run of `__int128` bit-fields, where skipping the unnamed field +/// leaves part of a wider access unit unclassified.) ABI_VER_MAJOR(23) /// Conform to the underlying platform's C and C++ ABIs as closely as we can. diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 17b5f0fe4133d..e0adfcc51c089 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -433,6 +433,7 @@ CodeGenModule::getLLVMABITargetInfo(llvm::abi::TypeBuilder &TB) { Compat > LangOptions::ClangABI::Ver20 && !T.isPS(); CompatInfo.Clang11Compat = Compat <= LangOptions::ClangABI::Ver11 || T.isPS(); + CompatInfo.Clang23Compat = Compat <= LangOptions::ClangABI::Ver23; bool Has64BitPointers = getTarget().getPointerWidth(LangAS::Default) == 64; diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index d60d71775a9d1..6a651e97902a3 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -2200,6 +2200,8 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo, bool UseClang11Compat = getContext().getLangOpts().isCompatibleWith( LangOptions::ClangABI::Ver11) || getContext().getTargetInfo().getTriple().isPS(); + bool UseClang23Compat = getContext().getLangOpts().isCompatibleWith( + LangOptions::ClangABI::Ver23); bool IsUnion = RT->isUnionType() && !UseClang11Compat; for (RecordDecl::field_iterator i = RD->field_begin(), e = RD->field_end(); @@ -2207,9 +2209,14 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo, uint64_t Offset = OffsetBase + Layout.getFieldOffset(idx); bool BitField = i->isBitField(); - // Ignore zero-length bit-fields. Other unnamed bit-fields are real - // storage and classify like named ones, matching GCC. - if (BitField && i->isZeroLengthBitField()) + // Ignore padding bit-fields. Under -fclang-abi-compat=23 every unnamed + // bit-field is padding, faithfully reproducing Clang 23 (including its + // crash on aggregates where doing so leaves part of a wider access unit, + // e.g. an __int128 bit-field run, unclassified); otherwise only + // zero-length bit-fields are, and other unnamed bit-fields classify like + // named ones, matching GCC. + if (BitField && (UseClang23Compat ? i->isUnnamedBitField() + : i->isZeroLengthBitField())) continue; // AMD64-ABI 3.2.3p2: Rule 1. If the size of an object is larger than @@ -2250,7 +2257,8 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo, // structure to be passed in memory even if unaligned, and // therefore they can straddle an eightbyte. if (BitField) { - assert(!i->isZeroLengthBitField()); + assert(UseClang23Compat ? !i->isUnnamedBitField() + : !i->isZeroLengthBitField()); uint64_t Offset = OffsetBase + Layout.getFieldOffset(idx); uint64_t Size = i->getBitWidthValue(); diff --git a/clang/test/CodeGen/X86/x86_64-unnamed-bitfield-abi-compat.c b/clang/test/CodeGen/X86/x86_64-unnamed-bitfield-abi-compat.c new file mode 100644 index 0000000000000..31df6b7553585 --- /dev/null +++ b/clang/test/CodeGen/X86/x86_64-unnamed-bitfield-abi-compat.c @@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | \ +// RUN: FileCheck %s -check-prefix=NEW +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fclang-abi-compat=23 %s -o - | \ +// RUN: FileCheck %s -check-prefix=COMPAT + +// A non-zero-width unnamed bit-field classifies the eightbyte it occupies as +// INTEGER like a named one, matching GCC, so this struct travels in two integer +// registers. Under -fclang-abi-compat=23 the unnamed bit-field is padding, so +// the low eightbyte stays NO_CLASS and only the high eightbyte is passed. +struct s { + long : 64; + long a; +}; + +// NEW-LABEL: define{{.*}} { i64, i64 } @get() +// COMPAT-LABEL: define{{.*}} i64 @get() +struct s get(void) { + return (struct s){0}; +} + +// NEW-LABEL: define{{.*}} void @put(i64 %a.coerce0, i64 %a.coerce1) +// COMPAT-LABEL: define{{.*}} void @put(i64 %a.coerce) +void put(struct s a) { +} + +// Note: -fclang-abi-compat=23 faithfully reproduces Clang 23, so it also +// reproduces Clang 23's crash on a run of __int128 bit-fields (skipping the +// unnamed field leaves half of the i128 access unit unclassified). That shape +// is deliberately not exercised here; the corrected classification is covered +// in x86_64-arguments.c. diff --git a/llvm/include/llvm/ABI/TargetInfo.h b/llvm/include/llvm/ABI/TargetInfo.h index 8132de140064a..1433c59623909 100644 --- a/llvm/include/llvm/ABI/TargetInfo.h +++ b/llvm/include/llvm/ABI/TargetInfo.h @@ -46,11 +46,12 @@ struct ABICompatInfo { bool ClassifyIntegerMMXAsSSE : 1; bool HonorsRevision98 : 1; bool Clang11Compat : 1; + bool Clang23Compat : 1; ABICompatInfo() : PassInt128VectorsInMem(true), ReturnCXXRecordGreaterThan128InMem(true), ClassifyIntegerMMXAsSSE(true), HonorsRevision98(true), - Clang11Compat(true) {} + Clang11Compat(true), Clang23Compat(false) {} /// Return flags matching the ABI emitted by the given Clang major version. // TODO: fill in per-version flag overrides. diff --git a/llvm/lib/ABI/Targets/X86.cpp b/llvm/lib/ABI/Targets/X86.cpp index 88bfb8ad453cc..5bb69f55de2bd 100644 --- a/llvm/lib/ABI/Targets/X86.cpp +++ b/llvm/lib/ABI/Targets/X86.cpp @@ -557,9 +557,13 @@ void X86_64TargetInfo::classify(const Type *T, uint64_t OffsetBase, Class &Lo, uint64_t Offset = OffsetBase + Field.OffsetInBits; bool BitField = Field.IsBitField; - // Ignore zero-length bit-fields. Other unnamed bit-fields are real - // storage and classify like named ones, matching GCC. - if (BitField && Field.BitFieldWidth == 0) + // Ignore padding bit-fields. Under Clang 23 compatibility every unnamed + // bit-field is padding, faithfully reproducing Clang 23; otherwise only + // zero-length bit-fields are, and other unnamed bit-fields classify like + // named ones, matching GCC. + if (BitField && + (getABICompatInfo().Clang23Compat ? Field.IsUnnamedBitfield + : Field.BitFieldWidth == 0)) continue; if (Size > 128 && `````````` </details> https://github.com/llvm/llvm-project/pull/220788 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
