[PATCH] D114536: [X86][MS] Fix the wrong alignment of vector variable arguments on Win32

[Phoebe Wang via Phabricator via llvm-commits]

pengfei updated this revision to Diff 400382.
pengfei added a comment.
Herald added a subscriber: qcolombet.

Remove `IsVarArg` boolean.

> There are no cases when a vector passed on the stack is aligned to 16 bytes, it should always be four byte aligned.

Unfortunately, there is. See changes in win32-spill-xmm.ll.

The good news is it doesn't look like a reasonable test. See https://godbolt.org/z/hdsPTsbPW
MSVC doesn't pass a 512 bits argument in that way while Clang FE passes it by pointer.
It seems D12337 <https://reviews.llvm.org/D12337> isn't a valid patch. So we can ignore the change in win32-spill-xmm.ll.
We may need to consider to fix the imcompatible issue between Clang and MSVC, but it is not related to this patch anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114536/new/

https://reviews.llvm.org/D114536

Files:
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/vaargs-win32.ll
  llvm/test/CodeGen/X86/win32-spill-xmm.ll


Index: llvm/test/CodeGen/X86/win32-spill-xmm.ll
===================================================================
--- llvm/test/CodeGen/X86/win32-spill-xmm.ll
+++ llvm/test/CodeGen/X86/win32-spill-xmm.ll
@@ -4,7 +4,7 @@
 
 ; CHECK-LABEL: spill_ok
 ; CHECK: subl    $32, %esp
-; CHECK: movaps  %xmm3, (%esp)
+; CHECK: movups  %xmm3, (%esp)
 ; CHECK: movl    $0, 16(%esp)
 ; CHECK: calll   _bar
 define void @spill_ok(i32, <16 x float> *) {
Index: llvm/test/CodeGen/X86/vaargs-win32.ll
===================================================================
--- llvm/test/CodeGen/X86/vaargs-win32.ll
+++ llvm/test/CodeGen/X86/vaargs-win32.ll
@@ -34,11 +34,11 @@
   ret void
 }
 
-define <4 x i32> @foo(<4 x float> %0, ...) nounwind {
+define <4 x i32> @foo(<4 x float> inreg %0, ...) nounwind {
 ; MSVC-LABEL: foo:
 ; MSVC:       # %bb.0:
 ; MSVC-NEXT:    pushl %eax
-; MSVC-NEXT:    movaps 8(%esp), %xmm0
+; MSVC-NEXT:    movups 8(%esp), %xmm0
 ; MSVC-NEXT:    movups 24(%esp), %xmm1
 ; MSVC-NEXT:    cmpltps %xmm1, %xmm0
 ; MSVC-NEXT:    popl %eax
@@ -73,9 +73,9 @@
 ; MSVC:       # %bb.0:
 ; MSVC-NEXT:    subl $32, %esp
 ; MSVC-NEXT:    movaps {{.*#+}} xmm0 = [5.0E+0,6.0E+0,7.0E+0,8.0E+0]
-; MSVC-NEXT:    movaps %xmm0, 16(%esp)
+; MSVC-NEXT:    movups %xmm0, 16(%esp)
 ; MSVC-NEXT:    movaps {{.*#+}} xmm0 = [1.0E+0,2.0E+0,3.0E+0,4.0E+0]
-; MSVC-NEXT:    movaps %xmm0, (%esp)
+; MSVC-NEXT:    movups %xmm0, (%esp)
 ; MSVC-NEXT:    calll _foo
 ; MSVC-NEXT:    addl $32, %esp
 ; MSVC-NEXT:    retl
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -3563,10 +3563,15 @@
     MFI.setObjectSExt(FI, true);
   }
 
+  MaybeAlign Alignment;
+  if (Subtarget.isTargetWindowsMSVC() && !Subtarget.is64Bit()
+      && ValVT != MVT::f80)
+    Alignment = MaybeAlign(4);
   SDValue FIN = DAG.getFrameIndex(FI, PtrVT);
   SDValue Val = DAG.getLoad(
       ValVT, dl, Chain, FIN,
-      MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI));
+      MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI),
+      Alignment);
   return ExtendedInMem
              ? (VA.getValVT().isVector()
                     ? DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, VA.getValVT(), Val)
@@ -4085,9 +4090,14 @@
   if (isByVal)
     return CreateCopyOfByValArgument(Arg, PtrOff, Chain, Flags, DAG, dl);
 
+  MaybeAlign Alignment;
+  if (Subtarget.isTargetWindowsMSVC() && !Subtarget.is64Bit() &&
+      Arg.getSimpleValueType() != MVT::f80)
+    Alignment = MaybeAlign(4);
   return DAG.getStore(
       Chain, dl, Arg, PtrOff,
-      MachinePointerInfo::getStack(DAG.getMachineFunction(), LocMemOffset));
+      MachinePointerInfo::getStack(DAG.getMachineFunction(), LocMemOffset),
+      Alignment);
 }
 
 /// Emit a load of return address if tail call


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114536.400382.patch
Type: text/x-patch
Size: 2900 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220116/1c315a74/attachment.bin>