[PATCH] D83175: [X86] Fix a bug that when lowering byval argument

[LiuChen via Phabricator via llvm-commits]

LiuChen3 created this revision.
LiuChen3 added reviewers: craig.topper, LuoYuanke, loladiro.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

When an argument has 'byval' attribute  and should be passed on the stack according calling convention,
a stack copy would be emitted twice.  This will cause the real value will be put into stack where the pointer should be passed.


https://reviews.llvm.org/D83175

Files:
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/X86/win64-byval.ll


Index: llvm/test/CodeGen/X86/win64-byval.ll
===================================================================
--- llvm/test/CodeGen/X86/win64-byval.ll
+++ llvm/test/CodeGen/X86/win64-byval.ll
@@ -32,3 +32,31 @@
     call void @foo({ float, double }* byval %arg)
     ret void
 }
+
+declare void @foo2({ float, double }* byval, { float, double }* byval, { float, double }* byval, { float, double }* byval, { float, double }* byval, i64 %f)
+ at data = external constant { float, double }
+
+define void @test() {
+; CHECK-LABEL: @test
+; CHECK:      movq    (%rax), %rcx
+; CHECK-NEXT: movq    8(%rax), %rax
+; CHECK-NEXT: movq    %rax, 120(%rsp)
+; CHECK-NEXT: movq    %rcx, 112(%rsp)
+; CHECK-NEXT: movq    %rcx, 96(%rsp)
+; CHECK-NEXT: movq    %rax, 104(%rsp)
+; CHECK-NEXT: movq    %rcx, 80(%rsp)
+; CHECK-NEXT: movq    %rax, 88(%rsp)
+; CHECK-NEXT: movq    %rcx, 64(%rsp)
+; CHECK-NEXT: movq    %rax, 72(%rsp)
+; CHECK-NEXT: movq    %rax, 56(%rsp)
+; CHECK-NEXT: movq    %rcx, 48(%rsp)
+; CHECK-NEXT: leaq    48(%rsp), %rax
+; CHECK-NEXT: movq    %rax, 32(%rsp)
+; CHECK-NEXT: movq    $10, 40(%rsp)
+; CHECK-NEXT: leaq    112(%rsp), %rcx
+; CHECK-NEXT: leaq    96(%rsp), %rdx
+; CHECK-NEXT: leaq    80(%rsp), %r8
+; CHECK-NEXT: leaq    64(%rsp), %r9
+  call void @foo2({ float, double }* byval @G, { float, double }* byval @G, { float, double }* byval @G, { float, double }* byval @G, { float, double }* byval @G, i64 10)
+  ret void
+}
Index: llvm/lib/Target/X86/X86ISelLowering.h
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.h
+++ llvm/lib/Target/X86/X86ISelLowering.h
@@ -1436,7 +1436,7 @@
     SDValue LowerMemOpCallTo(SDValue Chain, SDValue StackPtr, SDValue Arg,
                              const SDLoc &dl, SelectionDAG &DAG,
                              const CCValAssign &VA,
-                             ISD::ArgFlagsTy Flags) const;
+                             ISD::ArgFlagsTy Flags, bool hasCopy) const;
 
     // Call lowering helpers.
 
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -3763,12 +3763,15 @@
                                             SDValue Arg, const SDLoc &dl,
                                             SelectionDAG &DAG,
                                             const CCValAssign &VA,
-                                            ISD::ArgFlagsTy Flags) const {
+                                            ISD::ArgFlagsTy Flags,
+                                            bool hasCopy) const {
   unsigned LocMemOffset = VA.getLocMemOffset();
   SDValue PtrOff = DAG.getIntPtrConstant(LocMemOffset, dl);
   PtrOff = DAG.getNode(ISD::ADD, dl, getPointerTy(DAG.getDataLayout()),
                        StackPtr, PtrOff);
-  if (Flags.isByVal())
+  // If the argument already has a copy on the stack, we do not need to
+  // creating a temporary stack slot, again.
+  if (Flags.isByVal() && !hasCopy)
     return CreateCopyOfByValArgument(Arg, PtrOff, Chain, Flags, DAG, dl);
 
   return DAG.getStore(
@@ -3997,6 +4000,7 @@
     EVT RegVT = VA.getLocVT();
     SDValue Arg = OutVals[OutIndex];
     bool isByVal = Flags.isByVal();
+    bool hasCopy = false;
 
     // Promote the value if needed.
     switch (VA.getLocInfo()) {
@@ -4038,6 +4042,7 @@
         // From now on treat this as a regular pointer
         Arg = StackSlot;
         isByVal = false;
+        hasCopy = true;
       } else {
         // Store the argument.
         SDValue SpillSlot = DAG.CreateStackTemporary(VA.getValVT());
@@ -4080,7 +4085,7 @@
         StackPtr = DAG.getCopyFromReg(Chain, dl, RegInfo->getStackRegister(),
                                       getPointerTy(DAG.getDataLayout()));
       MemOpChains.push_back(LowerMemOpCallTo(Chain, StackPtr, Arg,
-                                             dl, DAG, VA, Flags));
+                                             dl, DAG, VA, Flags, hasCopy));
     }
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83175.275547.patch
Type: text/x-patch
Size: 4077 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200705/1e924d78/attachment.bin>