[clang] 651e5ae - [MS] Fix passing aligned records by value in some cases

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2023-06-13T12:54:23-07:00
New Revision: 651e5ae62d29fdb07eb85f75ab7e686b98479f3a

URL: https://github.com/llvm/llvm-project/commit/651e5ae62d29fdb07eb85f75ab7e686b98479f3a
DIFF: https://github.com/llvm/llvm-project/commit/651e5ae62d29fdb07eb85f75ab7e686b98479f3a.diff

LOG: [MS] Fix passing aligned records by value in some cases

It's not exactly clear what the meaning of TypeInfo::AlignRequirement
is, so go directly to the ASTRecordLayout for records and check the
required alignment there. Compare that number with the stack alignment
value of 4.

This fixes cases when the alignment attribute does not appear directly
on the record [1], or when the attribute on the record is underaligned
[2].

[1]: `struct Foo { int __declspec(align(16)) x; };`
[2]: `struct __declspec(align(1)) Bar { int x; };`

Fixes https://llvm.org/pr63257

Differential Revision: https://reviews.llvm.org/D152752

Added: 
    

Modified: 
    clang/lib/CodeGen/TargetInfo.cpp
    clang/test/CodeGen/X86/x86_32-arguments-win32.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp
index 3b9cce40b2fb6..7a236875f2442 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -1893,9 +1893,21 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty,
     llvm::IntegerType *PaddingType = NeedsPadding ? Int32 : nullptr;
 
     // Pass over-aligned aggregates on Windows indirectly. This behavior was
-    // added in MSVC 2015.
-    if (IsWin32StructABI && TI.isAlignRequired() && TI.Align > 32)
-      return getIndirectResult(Ty, /*ByVal=*/false, State);
+    // 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) {
+      unsigned AlignInBits = 0;
+      if (RT) {
+        const ASTRecordLayout &Layout =
+          getContext().getASTRecordLayout(RT->getDecl());
+        AlignInBits = getContext().toBits(Layout.getRequiredAlignment());
+      } else if (TI.isAlignRequired()) {
+        AlignInBits = TI.Align;
+      }
+      if (AlignInBits > 32)
+        return getIndirectResult(Ty, /*ByVal=*/false, State);
+    }
 
     // Expand small (<= 128-bit) record types when we know that the stack layout
     // of those arguments will match the struct. This is important because the

diff  --git a/clang/test/CodeGen/X86/x86_32-arguments-win32.c b/clang/test/CodeGen/X86/x86_32-arguments-win32.c
index dc5058231b8ad..53f6b5b79642c 100644
--- a/clang/test/CodeGen/X86/x86_32-arguments-win32.c
+++ b/clang/test/CodeGen/X86/x86_32-arguments-win32.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -w -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fms-extensions -w -triple i386-pc-win32 -emit-llvm -o - %s | FileCheck %s
 
 // CHECK-LABEL: define dso_local i64 @f1_1()
 // CHECK-LABEL: define dso_local void @f1_2(i32 %a0.0, i32 %a0.1)
@@ -90,3 +90,41 @@ void __fastcall fastcall_indirect_vec(__m128 x, __m128 y, __m128 z, __m128 w, in
   gv128 = x + y + z + w + q;
 }
 // CHECK-LABEL: define dso_local x86_fastcallcc void @"\01 at fastcall_indirect_vec@84"(<4 x float> inreg noundef %x, <4 x float> inreg noundef %y, <4 x float> inreg noundef %z, ptr inreg noundef %0, i32 inreg noundef %edx, ptr noundef %1)
+
+struct __declspec(align(1)) Align1 { unsigned long long x; };
+struct __declspec(align(4)) Align4 { unsigned long long x; };
+struct __declspec(align(8)) Align8 { unsigned long long x; };
+void receive_align1(struct Align1 o);
+void receive_align4(struct Align4 o);
+void receive_align8(struct Align8 o);
+void pass_underaligned_record() {
+  struct Align1 a1;
+  receive_align1(a1);
+  struct Align4 a4;
+  receive_align4(a4);
+  struct Align8 a8;
+  receive_align8(a8);
+}
+// CHECK-LABEL: define dso_local void @pass_underaligned_record()
+// CHECK: call void @receive_align1(i64 {{[^,)]*}})
+// CHECK: call void @receive_align4(i64 {{[^,)]*}})
+// CHECK: call void @receive_align8(ptr {{[^,)]*}})
+
+struct FieldAlign1 { unsigned long long __declspec(align(1)) x; };
+struct FieldAlign4 { unsigned long long __declspec(align(4)) x; };
+struct FieldAlign8 { unsigned long long __declspec(align(8)) x; };
+void receive_falign1(struct FieldAlign1 o);
+void receive_falign4(struct FieldAlign4 o);
+void receive_falign8(struct FieldAlign8 o);
+void pass_underaligned_record_field() {
+  struct FieldAlign1 a1;
+  receive_falign1(a1);
+  struct FieldAlign4 a4;
+  receive_falign4(a4);
+  struct FieldAlign8 a8;
+  receive_falign8(a8);
+}
+// CHECK-LABEL: define dso_local void @pass_underaligned_record_field()
+// CHECK: call void @receive_falign1(i64 {{[^,)]*}})
+// CHECK: call void @receive_falign4(i64 {{[^,)]*}})
+// CHECK: call void @receive_falign8(ptr {{[^,)]*}})