https://github.com/akash-manna-sky updated https://github.com/llvm/llvm-project/pull/216777
>From 3c0b7687c5010dc7c031e50d2f363a2868b7bb42 Mon Sep 17 00:00:00 2001 From: Akash Manna <[email protected]> Date: Mon, 17 Aug 2026 22:03:28 +0530 Subject: [PATCH 1/2] [Clang][CodeGen][X86] Fix crash on __int128 bit-field access units --- clang/lib/CodeGen/Targets/X86.cpp | 26 +++++++++++++---------- clang/test/CodeGen/X86/x86_64-arguments.c | 26 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index f0d108f3279fd..db2733558147d 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -2628,12 +2628,10 @@ GetINTEGERTypeAtOffset(llvm::Type *IRType, unsigned IROffset, SourceOffset); } - // if we have a 128-bit integer, we can pass it safely using an i128 - // so we return that - if (IRType->isIntegerTy(128)) { - assert(IROffset == 0); + // A 128-bit integer can be passed safely as an i128, but only if it starts at + // this offset; otherwise it spans more than the eightbyte we're describing. + if (IRType->isIntegerTy(128) && IROffset == 0) return IRType; - } // Okay, we don't have any better idea of what to pass, so we pass this in an // integer register that isn't too big to fit the rest of the struct. @@ -2739,10 +2737,13 @@ ABIArgInfo X86_64ABIInfo::classifyReturnType(QualType RetTy) const { return ABIArgInfo::getExtend(RetTy); } + // An i128 covers both eightbytes, so it only describes the value if the + // high eightbyte is INTEGER too; otherwise it is bit-field storage whose + // high eightbyte holds no data. if (ResType->isIntegerTy(128)) { - // i128 are passed directly - assert(Hi == Integer); - return ABIArgInfo::getDirect(ResType); + if (Hi == Integer) + return ABIArgInfo::getDirect(ResType); + ResType = llvm::Type::getInt64Ty(getVMContext()); } break; @@ -2889,10 +2890,13 @@ X86_64ABIInfo::classifyArgumentType(QualType Ty, unsigned freeIntRegs, return ABIArgInfo::getExtend(Ty, CGT.ConvertType(Ty)); } + // See the matching comment in classifyReturnType. if (ResType->isIntegerTy(128)) { - assert(Hi == Integer); - ++neededInt; - return ABIArgInfo::getDirect(ResType); + if (Hi == Integer) { + ++neededInt; + return ABIArgInfo::getDirect(ResType); + } + ResType = llvm::Type::getInt64Ty(getVMContext()); } break; diff --git a/clang/test/CodeGen/X86/x86_64-arguments.c b/clang/test/CodeGen/X86/x86_64-arguments.c index 580f9487395d3..9f5709e6838f8 100644 --- a/clang/test/CodeGen/X86/x86_64-arguments.c +++ b/clang/test/CodeGen/X86/x86_64-arguments.c @@ -590,6 +590,32 @@ _BitInt(128) f74(__uint128_t b, __uint128_t c, __uint128_t d, long e, _BitInt(12 return a; } +// check that a run of (u)int128_t bit-fields, which is lowered to an i128 +// access unit, is not passed as an i128 when only one eightbyte is INTEGER +struct s75 { + __uint128_t : 124; + __uint128_t a : 4; +}; +// CHECK-LABEL: define{{.*}} i64 @f75() +struct s75 f75(void) { + return (struct s75){0}; +} +// CHECK-LABEL: define{{.*}} void @f76(i64 %a.coerce) +void f76(struct s75 a) { +} + +struct s77 { + __uint128_t a : 4; + __uint128_t : 124; +}; +// CHECK-LABEL: define{{.*}} i64 @f77() +struct s77 f77(void) { + return (struct s77){0}; +} +// CHECK-LABEL: define{{.*}} void @f78(i64 %a.coerce) +void f78(struct s77 a) { +} + /// The synthesized __va_list_tag does not have file/line fields. // CHECK: = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "__va_list_tag", // CHECK-NOT: file: >From 859db871f89c15fe4c2269fac6225bf961d9503d Mon Sep 17 00:00:00 2001 From: Akash Manna <[email protected]> Date: Fri, 21 Aug 2026 23:30:01 +0530 Subject: [PATCH 2/2] [Clang][CodeGen][X86] Fix crash on __int128 bit-field access units The x86-64 SysV classifier skipped all unnamed bit-fields as padding, so an eightbyte occupied only by a non-zero-width unnamed bit-field stayed NO_CLASS. GCC classifies such storage as INTEGER. Besides diverging from GCC for aggregates like struct { long : 64; long a; }, this crashed on a run of __int128 bit-fields: record lowering merges the run into one i128 access unit spanning both eightbytes, and with only one of them INTEGER the i128 was queried at offset 8 (tripping assert(IROffset == 0) in GetINTEGERTypeAtOffset) or returned while the high eightbyte was NO_CLASS (tripping assert(Hi == Integer) in the callers); release builds emitted a 16-byte load at offset 8 of the 16-byte value. Skip only zero-length bit-fields, which occupy no storage, and classify the rest like named ones, matching GCC. Both eightbytes of an i128 bit-field run then classify INTEGER, so the existing i128 asserts hold. Apply the same change to the experimental ABI library. Fixes #202205 --- clang/docs/ReleaseNotes.md | 7 +++++ clang/lib/CodeGen/Targets/X86.cpp | 33 +++++++++----------- clang/test/CodeGen/X86/x86_64-arguments.c | 38 +++++++++++++++++++---- clang/test/CodeGen/X86/x86_64-union-abi.c | 19 ++++++++++++ llvm/lib/ABI/Targets/X86.cpp | 4 ++- 5 files changed, 76 insertions(+), 25 deletions(-) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index d9b9c92950c98..3f590cbf22794 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -86,6 +86,13 @@ features cannot lower the translation-unit ABI level; always passed the parts separately. `-fclang-abi-compat=23` restores the previous behavior. (#GH212109) +- On x86-64 System V, a non-zero-width unnamed bit-field now classifies the + eightbytes it occupies as INTEGER, like a named bit-field, matching GCC. + 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) + ### AST Dumping Potentially Breaking Changes ### Clang Frontend Potentially Breaking Changes diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index db2733558147d..2f18b265bc50a 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -2204,8 +2204,9 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo, uint64_t Offset = OffsetBase + Layout.getFieldOffset(idx); bool BitField = i->isBitField(); - // Ignore padding bit-fields. - if (BitField && i->isUnnamedBitField()) + // Ignore zero-length bit-fields. Other unnamed bit-fields are real + // storage and classify like named ones, matching GCC. + if (BitField && i->isZeroLengthBitField()) continue; // AMD64-ABI 3.2.3p2: Rule 1. If the size of an object is larger than @@ -2246,7 +2247,7 @@ 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->isUnnamedBitField()); + assert(!i->isZeroLengthBitField()); uint64_t Offset = OffsetBase + Layout.getFieldOffset(idx); uint64_t Size = i->getBitWidthValue(); @@ -2628,10 +2629,12 @@ GetINTEGERTypeAtOffset(llvm::Type *IRType, unsigned IROffset, SourceOffset); } - // A 128-bit integer can be passed safely as an i128, but only if it starts at - // this offset; otherwise it spans more than the eightbyte we're describing. - if (IRType->isIntegerTy(128) && IROffset == 0) + // if we have a 128-bit integer, we can pass it safely using an i128 + // so we return that + if (IRType->isIntegerTy(128)) { + assert(IROffset == 0); return IRType; + } // Okay, we don't have any better idea of what to pass, so we pass this in an // integer register that isn't too big to fit the rest of the struct. @@ -2737,13 +2740,10 @@ ABIArgInfo X86_64ABIInfo::classifyReturnType(QualType RetTy) const { return ABIArgInfo::getExtend(RetTy); } - // An i128 covers both eightbytes, so it only describes the value if the - // high eightbyte is INTEGER too; otherwise it is bit-field storage whose - // high eightbyte holds no data. if (ResType->isIntegerTy(128)) { - if (Hi == Integer) - return ABIArgInfo::getDirect(ResType); - ResType = llvm::Type::getInt64Ty(getVMContext()); + // i128 are passed directly + assert(Hi == Integer); + return ABIArgInfo::getDirect(ResType); } break; @@ -2890,13 +2890,10 @@ X86_64ABIInfo::classifyArgumentType(QualType Ty, unsigned freeIntRegs, return ABIArgInfo::getExtend(Ty, CGT.ConvertType(Ty)); } - // See the matching comment in classifyReturnType. if (ResType->isIntegerTy(128)) { - if (Hi == Integer) { - ++neededInt; - return ABIArgInfo::getDirect(ResType); - } - ResType = llvm::Type::getInt64Ty(getVMContext()); + assert(Hi == Integer); + ++neededInt; + return ABIArgInfo::getDirect(ResType); } break; diff --git a/clang/test/CodeGen/X86/x86_64-arguments.c b/clang/test/CodeGen/X86/x86_64-arguments.c index 9f5709e6838f8..b56792dd50cdd 100644 --- a/clang/test/CodeGen/X86/x86_64-arguments.c +++ b/clang/test/CodeGen/X86/x86_64-arguments.c @@ -590,17 +590,17 @@ _BitInt(128) f74(__uint128_t b, __uint128_t c, __uint128_t d, long e, _BitInt(12 return a; } -// check that a run of (u)int128_t bit-fields, which is lowered to an i128 -// access unit, is not passed as an i128 when only one eightbyte is INTEGER +// check that non-zero-width unnamed bit-fields classify INTEGER like named +// ones, so a run of (u)int128_t bit-fields is passed and returned as an i128 struct s75 { __uint128_t : 124; __uint128_t a : 4; }; -// CHECK-LABEL: define{{.*}} i64 @f75() +// CHECK-LABEL: define{{.*}} i128 @f75() struct s75 f75(void) { return (struct s75){0}; } -// CHECK-LABEL: define{{.*}} void @f76(i64 %a.coerce) +// CHECK-LABEL: define{{.*}} void @f76(i128 %a.coerce) void f76(struct s75 a) { } @@ -608,14 +608,40 @@ struct s77 { __uint128_t a : 4; __uint128_t : 124; }; -// CHECK-LABEL: define{{.*}} i64 @f77() +// CHECK-LABEL: define{{.*}} i128 @f77() struct s77 f77(void) { return (struct s77){0}; } -// CHECK-LABEL: define{{.*}} void @f78(i64 %a.coerce) +// CHECK-LABEL: define{{.*}} void @f78(i128 %a.coerce) void f78(struct s77 a) { } +// an unnamed bit-field filling the low eightbyte makes it INTEGER +struct s79 { + long : 64; + long a; +}; +// CHECK-LABEL: define{{.*}} { i64, i64 } @f79() +struct s79 f79(void) { + return (struct s79){0}; +} +// CHECK-LABEL: define{{.*}} void @f80(i64 %a.coerce0, i64 %a.coerce1) +void f80(struct s79 a) { +} + +// an unnamed bit-field in the high eightbyte is INTEGER while the low is SSE +struct s81 { + double d; + int : 32; +}; +// CHECK-LABEL: define{{.*}} { double, i32 } @f81() +struct s81 f81(void) { + return (struct s81){0}; +} +// CHECK-LABEL: define{{.*}} void @f82(double %a.coerce0, i32 %a.coerce1) +void f82(struct s81 a) { +} + /// The synthesized __va_list_tag does not have file/line fields. // CHECK: = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "__va_list_tag", // CHECK-NOT: file: diff --git a/clang/test/CodeGen/X86/x86_64-union-abi.c b/clang/test/CodeGen/X86/x86_64-union-abi.c index a9b5c60f7eeaa..3bc60464c02a8 100644 --- a/clang/test/CodeGen/X86/x86_64-union-abi.c +++ b/clang/test/CodeGen/X86/x86_64-union-abi.c @@ -76,3 +76,22 @@ void take_wide_unnamed(union WideUnnamedBitfield u); void call_wide_unnamed(union WideUnnamedBitfield u) { take_wide_unnamed(u); } // CHECK-DAG: declare void @take_wide_unnamed(i64) + +// A non-zero-width unnamed bitfield is INTEGER, which beats the double's SSE +// in the merge, so the union travels in a GPR. +union DoubleUnnamedBitfield { + double d; + long : 64; +}; + +void take_double_unnamed(union DoubleUnnamedBitfield u); +void call_double_unnamed(union DoubleUnnamedBitfield u) { + take_double_unnamed(u); +} + +// CHECK-DAG: declare void @take_double_unnamed(i64) + +union DoubleUnnamedBitfield ret_double_unnamed(void); +void call_ret_double_unnamed(void) { ret_double_unnamed(); } + +// CHECK-DAG: declare i64 @ret_double_unnamed() diff --git a/llvm/lib/ABI/Targets/X86.cpp b/llvm/lib/ABI/Targets/X86.cpp index 83c13f0071f23..42816808048f1 100644 --- a/llvm/lib/ABI/Targets/X86.cpp +++ b/llvm/lib/ABI/Targets/X86.cpp @@ -545,7 +545,9 @@ void X86_64TargetInfo::classify(const Type *T, uint64_t OffsetBase, Class &Lo, uint64_t Offset = OffsetBase + Field.OffsetInBits; bool BitField = Field.IsBitField; - if (BitField && Field.IsUnnamedBitfield) + // Ignore zero-length bit-fields. Other unnamed bit-fields are real + // storage and classify like named ones, matching GCC. + if (BitField && Field.BitFieldWidth == 0) continue; if (Size > 128 && _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
