From: Reid Kleckner <rnk@google.com>
Date: Mon, 6 Mar 2017 18:39:39 +0000 (+0000)
Subject: [X86] Fix arg copy elision for illegal types
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=08e9d0ec12eb5c1dc6f58b74c5314525fe639224;p=llvm

[X86] Fix arg copy elision for illegal types

Use the store size of the argument type, which will be a byte-sized
quantity, rather than dividing the size in bits by 8.

Fixes PR32136 and re-enables copy elision from i64 arguments.

Reverts the workaround in from r296950.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@297045 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
index 7a023fb212b..88870e51f45 100644
--- a/lib/Target/X86/X86ISelLowering.cpp
+++ b/lib/Target/X86/X86ISelLowering.cpp
@@ -2744,44 +2744,40 @@ X86TargetLowering::LowerMemArgument(SDValue Chain, CallingConv::ID CallConv,
   // This is an argument in memory. We might be able to perform copy elision.
   if (Flags.isCopyElisionCandidate()) {
     EVT ArgVT = Ins[i].ArgVT;
-    if (isTypeLegal(ArgVT)) {
-      SDValue PartAddr;
-      if (Ins[i].PartOffset == 0) {
-        // If this is a one-part value or the first part of a multi-part value,
-        // create a stack object for the entire argument value type and return a
-        // load from our portion of it. This assumes that if the first part of
-        // an argument is in memory, the rest will also be in memory.
-        unsigned SizeInBits = ArgVT.getSizeInBits();
-        assert(SizeInBits % 8 == 0);
-        int FI = MFI.CreateFixedObject(SizeInBits / 8, VA.getLocMemOffset(),
-                                       /*Immutable=*/false);
-        PartAddr = DAG.getFrameIndex(FI, PtrVT);
+    SDValue PartAddr;
+    if (Ins[i].PartOffset == 0) {
+      // If this is a one-part value or the first part of a multi-part value,
+      // create a stack object for the entire argument value type and return a
+      // load from our portion of it. This assumes that if the first part of an
+      // argument is in memory, the rest will also be in memory.
+      int FI = MFI.CreateFixedObject(ArgVT.getStoreSize(), VA.getLocMemOffset(),
+                                     /*Immutable=*/false);
+      PartAddr = DAG.getFrameIndex(FI, PtrVT);
+      return DAG.getLoad(
+          ValVT, dl, Chain, PartAddr,
+          MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI));
+    } else {
+      // This is not the first piece of an argument in memory. See if there is
+      // already a fixed stack object including this offset. If so, assume it
+      // was created by the PartOffset == 0 branch above and create a load from
+      // the appropriate offset into it.
+      int64_t PartBegin = VA.getLocMemOffset();
+      int64_t PartEnd = PartBegin + ValVT.getSizeInBits() / 8;
+      int FI = MFI.getObjectIndexBegin();
+      for (; MFI.isFixedObjectIndex(FI); ++FI) {
+        int64_t ObjBegin = MFI.getObjectOffset(FI);
+        int64_t ObjEnd = ObjBegin + MFI.getObjectSize(FI);
+        if (ObjBegin <= PartBegin && PartEnd <= ObjEnd)
+          break;
+      }
+      if (MFI.isFixedObjectIndex(FI)) {
+        SDValue Addr =
+            DAG.getNode(ISD::ADD, dl, PtrVT, DAG.getFrameIndex(FI, PtrVT),
+                        DAG.getIntPtrConstant(Ins[i].PartOffset, dl));
         return DAG.getLoad(
-            ValVT, dl, Chain, PartAddr,
-            MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI));
-      } else {
-        // This is not the first piece of an argument in memory. See if there is
-        // already a fixed stack object including this offset. If so, assume it
-        // was created by the PartOffset == 0 branch above and create a load
-        // from the appropriate offset into it.
-        int64_t PartBegin = VA.getLocMemOffset();
-        int64_t PartEnd = PartBegin + ValVT.getSizeInBits() / 8;
-        int FI = MFI.getObjectIndexBegin();
-        for (; MFI.isFixedObjectIndex(FI); ++FI) {
-          int64_t ObjBegin = MFI.getObjectOffset(FI);
-          int64_t ObjEnd = ObjBegin + MFI.getObjectSize(FI);
-          if (ObjBegin <= PartBegin && PartEnd <= ObjEnd)
-            break;
-        }
-        if (MFI.isFixedObjectIndex(FI)) {
-          SDValue Addr =
-              DAG.getNode(ISD::ADD, dl, PtrVT, DAG.getFrameIndex(FI, PtrVT),
-                          DAG.getIntPtrConstant(Ins[i].PartOffset, dl));
-          return DAG.getLoad(
-              ValVT, dl, Chain, Addr,
-              MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI,
-                                                Ins[i].PartOffset));
-        }
+            ValVT, dl, Chain, Addr,
+            MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI,
+                                              Ins[i].PartOffset));
       }
     }
   }
diff --git a/test/CodeGen/X86/arg-copy-elide.ll b/test/CodeGen/X86/arg-copy-elide.ll
index 939c2f03e2b..b9a2eeeb7f8 100644
--- a/test/CodeGen/X86/arg-copy-elide.ll
+++ b/test/CodeGen/X86/arg-copy-elide.ll
@@ -60,10 +60,8 @@ entry:
 ; CHECK: andl $-8, %esp
 ; CHECK-DAG: movl 8(%ebp), %[[csr1]]
 ; CHECK-DAG: movl 12(%ebp), %[[csr2]]
-; CHECK: movl %edi, 4(%esp)
-; CHECK: movl %esi, (%esp)
-; CEHCK: movl %esp, %eax
-; CHECK: pushl %eax
+; CHECK-DAG: leal 8(%ebp), %[[reg:[^ ]*]]
+; CHECK: pushl %[[reg]]
 ; CHECK: calll _addrof_i64
 ; CHECK-DAG: movl %[[csr1]], %eax
 ; CHECK-DAG: movl %[[csr2]], %edx