[llvm] r297045 - [X86] Fix arg copy elision for illegal types

[Reid Kleckner via llvm-commits]

Author: rnk
Date: Mon Mar  6 12:39:39 2017
New Revision: 297045

URL: http://llvm.org/viewvc/llvm-project?rev=297045&view=rev
Log:
[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.

Modified:
    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
    llvm/trunk/test/CodeGen/X86/arg-copy-elide.ll

Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=297045&r1=297044&r2=297045&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Mon Mar  6 12:39:39 2017
@@ -2744,44 +2744,40 @@ X86TargetLowering::LowerMemArgument(SDVa
   // 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));
       }
     }
   }

Modified: llvm/trunk/test/CodeGen/X86/arg-copy-elide.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/arg-copy-elide.ll?rev=297045&r1=297044&r2=297045&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/arg-copy-elide.ll (original)
+++ llvm/trunk/test/CodeGen/X86/arg-copy-elide.ll Mon Mar  6 12:39:39 2017
@@ -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