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

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: nikic, hans.
Herald added subscribers: StephenFan, pengfei.
Herald added a project: All.
rnk requested review of this revision.
Herald added a project: clang.

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; };`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152752

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


Index: clang/test/CodeGen/X86/x86_32-arguments-win32.c
===================================================================
--- clang/test/CodeGen/X86/x86_32-arguments-win32.c
+++ 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 @@
   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 {{[^,)]*}})
Index: clang/lib/CodeGen/TargetInfo.cpp
===================================================================
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1893,9 +1893,21 @@
     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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152752.530661.patch
Type: text/x-patch
Size: 3642 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230612/15d16fb0/attachment-0001.bin>