https://github.com/rnk updated https://github.com/llvm/llvm-project/pull/65692:
>From 98d560c8057b171c81b43d93c1a0c26f1d27cf5b Mon Sep 17 00:00:00 2001 From: Reid Kleckner <r...@ad.corp.google.com> Date: Tue, 29 Aug 2023 14:26:10 -0700 Subject: [PATCH 1/2] [MS] Follow up fix to pass aligned args to variadic x86_32 functions MSVC allows users to pass structures with required alignments greater than 4 to variadic functions. It does not pass them indirectly to correctly align them. Instead, it passes them directly with the usual 4 byte stack alignment. This change implements the same logic in clang on the passing side. The receiving side (va_arg) never implemented any of this indirect logic, so it doesn't need to be updated. This issue pre-existed, but @aaron.ballman noticed it when we started passing structs containing aligned fields indirectly in D152752. --- clang/lib/CodeGen/Targets/X86.cpp | 27 ++++++++++------- .../test/CodeGen/X86/x86_32-arguments-win32.c | 29 +++++++++++++++++++ 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index 9f5c3258d65cb47..f04e0c67a807e1a 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -87,12 +87,15 @@ static ABIArgInfo getDirectX86Hva(llvm::Type* T = nullptr) { /// Similar to llvm::CCState, but for Clang. struct CCState { CCState(CGFunctionInfo &FI) - : IsPreassigned(FI.arg_size()), CC(FI.getCallingConvention()) {} + : IsPreassigned(FI.arg_size()), CC(FI.getCallingConvention()), + Required(FI.getRequiredArgs()), IsDelegateCall(FI.isDelegateCall()) {} llvm::SmallBitVector IsPreassigned; unsigned CC = CallingConv::CC_C; unsigned FreeRegs = 0; unsigned FreeSSERegs = 0; + RequiredArgs Required; + bool IsDelegateCall = false; }; /// X86_32ABIInfo - The X86-32 ABI information. @@ -141,7 +144,7 @@ class X86_32ABIInfo : public ABIInfo { Class classify(QualType Ty) const; ABIArgInfo classifyReturnType(QualType RetTy, CCState &State) const; ABIArgInfo classifyArgumentType(QualType RetTy, CCState &State, - bool isDelegateCall) const; + unsigned ArgIndex) const; /// Updates the number of available free registers, returns /// true if any registers were allocated. @@ -739,7 +742,7 @@ void X86_32ABIInfo::runVectorCallFirstPass(CGFunctionInfo &FI, CCState &State) c } ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty, CCState &State, - bool isDelegateCall) const { + unsigned ArgIndex) const { // FIXME: Set alignment on indirect arguments. bool IsFastCall = State.CC == llvm::CallingConv::X86_FastCall; bool IsRegCall = State.CC == llvm::CallingConv::X86_RegCall; @@ -754,7 +757,7 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty, CCState &State, CGCXXABI::RecordArgABI RAA = getRecordArgABI(RT, getCXXABI()); if (RAA == CGCXXABI::RAA_Indirect) { return getIndirectResult(Ty, false, State); - } else if (isDelegateCall) { + } else if (State.IsDelegateCall) { // Avoid having different alignments on delegate call args by always // setting the alignment to 4, which is what we do for inallocas. ABIArgInfo Res = getIndirectResult(Ty, false, State); @@ -812,11 +815,13 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty, CCState &State, } llvm::IntegerType *PaddingType = NeedsPadding ? Int32 : nullptr; - // Pass over-aligned aggregates on Windows indirectly. This behavior was - // added in MSVC 2015. Use the required alignment from the record layout, - // since that may be less than the regular type alignment, and types with - // required alignment of less than 4 bytes are not passed indirectly. - if (IsWin32StructABI) { + // Pass over-aligned aggregates to non-variadic functions on Windows + // indirectly. This behavior was added in MSVC 2015. Use the required + // alignment from the record layout, since that may be less than the + // regular type alignment, and types with required alignment of less than 4 + // bytes are not passed indirectly. + if (IsWin32StructABI && (!State.Required.allowsOptionalArgs() || + ArgIndex < State.Required.getNumRequiredArgs())) { unsigned AlignInBits = 0; if (RT) { const ASTRecordLayout &Layout = @@ -942,13 +947,13 @@ void X86_32ABIInfo::computeInfo(CGFunctionInfo &FI) const { bool UsedInAlloca = false; MutableArrayRef<CGFunctionInfoArgInfo> Args = FI.arguments(); - for (int I = 0, E = Args.size(); I < E; ++I) { + for (unsigned I = 0, E = Args.size(); I < E; ++I) { // Skip arguments that have already been assigned. if (State.IsPreassigned.test(I)) continue; Args[I].info = - classifyArgumentType(Args[I].type, State, FI.isDelegateCall()); + classifyArgumentType(Args[I].type, State, I); UsedInAlloca |= (Args[I].info.getKind() == ABIArgInfo::InAlloca); } diff --git a/clang/test/CodeGen/X86/x86_32-arguments-win32.c b/clang/test/CodeGen/X86/x86_32-arguments-win32.c index 53f6b5b79642c37..5b81c43f4bbb878 100644 --- a/clang/test/CodeGen/X86/x86_32-arguments-win32.c +++ b/clang/test/CodeGen/X86/x86_32-arguments-win32.c @@ -128,3 +128,32 @@ void pass_underaligned_record_field() { // CHECK: call void @receive_falign1(i64 {{[^,)]*}}) // CHECK: call void @receive_falign4(i64 {{[^,)]*}}) // CHECK: call void @receive_falign8(ptr {{[^,)]*}}) + +struct __declspec(align(8)) BigAligned { + int big[5]; +}; + +void receive_aligned_variadic(int f, ...); +void pass_aligned_variadic() { + struct Align8 a8 = {42}; + struct FieldAlign8 f8 = {42}; + struct BigAligned big; + receive_aligned_variadic(1, a8, f8, big); +} +// MSVC doesn't pass aligned objects to variadic functions indirectly. +// CHECK-LABEL: define dso_local void @pass_aligned_variadic() +// CHECK: call void (i32, ...) @receive_aligned_variadic(i32 noundef 1, i64 %{{[^,]*}}, i64 %{{[^,]*}}, ptr noundef byval(%struct.BigAligned) align 4 %{{[^)]*}}) + + +void receive_fixed_align_variadic(struct BigAligned big, ...); +void pass_fixed_align_variadic() { + struct BigAligned big; + receive_fixed_align_variadic(big, 42); +} +// MSVC emits error C2719 and C3916 when receiving and passing arguments with +// required alignment greater than 4 to the fixed part of a variadic function +// prototype, but it's actually easier to just implement this functionality +// correctly in Clang than it is to be bug for bug compatible, so we pass such +// arguments indirectly. +// CHECK-LABEL: define dso_local void @pass_fixed_align_variadic() +// CHECK: call void (ptr, ...) @receive_fixed_align_variadic(ptr noundef %{{[^)]*}}, i32 noundef 42) >From d77368b0475ac07eb9eaaad15729270d9c5acf05 Mon Sep 17 00:00:00 2001 From: Reid Kleckner <r...@google.com> Date: Wed, 13 Sep 2023 16:26:04 -0700 Subject: [PATCH 2/2] Add RequiredArgs helper --- clang/include/clang/CodeGen/CGFunctionInfo.h | 5 +++++ clang/lib/CodeGen/Targets/X86.cpp | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/CodeGen/CGFunctionInfo.h b/clang/include/clang/CodeGen/CGFunctionInfo.h index b8971d5793f3618..e388901b8a504c5 100644 --- a/clang/include/clang/CodeGen/CGFunctionInfo.h +++ b/clang/include/clang/CodeGen/CGFunctionInfo.h @@ -527,6 +527,11 @@ class RequiredArgs { return NumRequired; } + /// Return true if the argument at a given index is required. + bool isRequiredArg(unsigned argIdx) const { + return argIdx == ~0U || argIdx < NumRequired; + } + unsigned getOpaqueData() const { return NumRequired; } static RequiredArgs getFromOpaqueData(unsigned value) { if (value == ~0U) return All; diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index 93437ea76b22104..b2e2f6789cce328 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -820,8 +820,7 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty, CCState &State, // alignment from the record layout, since that may be less than the // regular type alignment, and types with required alignment of less than 4 // bytes are not passed indirectly. - if (IsWin32StructABI && (!State.Required.allowsOptionalArgs() || - ArgIndex < State.Required.getNumRequiredArgs())) { + if (IsWin32StructABI && State.Required.isRequiredArg(ArgIndex)) { unsigned AlignInBits = 0; if (RT) { const ASTRecordLayout &Layout = _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits