llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Akash Manna (akash-manna-sky) <details> <summary>Changes</summary> Clang crashed on `assert(IROffset == 0)` in `X86_64ABIInfo::GetINTEGERTypeAtOffset` when a struct containing a run of `__int128` bit-fields was passed or returned under the x86-64 SysV ABI: record lowering merges such bit-fields into a single `i128` access unit, but ABI classification ignores unnamed (padding) bit-fields and can therefore assign only one of the two eightbytes to the INTEGER class — so the `i128` gets queried at offset 8, or is returned as the low part while the high eightbyte is NO_CLASS (which trips the companion `assert(Hi == Integer)` in the callers and, without assertions, passes the value in a register pair the ABI never assigned). This patch takes the `i128` shortcut only when the `i128` starts at the requested offset and the high eightbyte is INTEGER too, otherwise falling back to the existing single-eightbyte handling that describes the value as an `i64` — restoring the pre-d35931c49e5b lowering — and adds regression tests for both field orders in return and argument position to `clang/test/CodeGen/X86/x86_64-arguments.c`. Fixes #<!-- -->202205. --- Full diff: https://github.com/llvm/llvm-project/pull/216777.diff 2 Files Affected: - (modified) clang/lib/CodeGen/Targets/X86.cpp (+15-11) - (modified) clang/test/CodeGen/X86/x86_64-arguments.c (+26) ``````````diff 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: `````````` </details> https://github.com/llvm/llvm-project/pull/216777 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
