[clang] [MS] Follow up fix to pass aligned args to variadic x86_32 functions (PR #65692)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 13 16:27:38 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
<details>
<summary>Changes</summary>
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.
--
Full diff: https://github.com/llvm/llvm-project/pull/65692.diff
3 Files Affected:
- (modified) clang/include/clang/CodeGen/CGFunctionInfo.h (+5)
- (modified) clang/lib/CodeGen/Targets/X86.cpp (+15-11)
- (modified) clang/test/CodeGen/X86/x86_32-arguments-win32.c (+29)
<pre>
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 2ea82397f11909e..b2e2f6789cce328 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,12 @@ 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.isRequiredArg(ArgIndex)) {
unsigned AlignInBits = 0;
if (RT) {
const ASTRecordLayout &Layout =
@@ -942,13 +946,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)
</pre>
</details>
https://github.com/llvm/llvm-project/pull/65692
More information about the cfe-commits
mailing list