From: Matthias Braun Date: Sat, 4 Mar 2017 01:40:40 +0000 (+0000) Subject: X86ISelLowering: Only perform copy elision on legal types. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dafdf041746861fb2d707d119110aeb200e53449;p=llvm X86ISelLowering: Only perform copy elision on legal types. This fixes cases where i1 types were not properly legalized yet and lead to the creating of 0-sized stack slots. This fixes http://llvm.org/PR32136 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@296950 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp index 317edc419e7..3993e47a051 100644 --- a/lib/Target/X86/X86ISelLowering.cpp +++ b/lib/Target/X86/X86ISelLowering.cpp @@ -2736,40 +2736,44 @@ 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; - 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.getSizeInBits() / 8, - 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)); + 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); return DAG.getLoad( - ValVT, dl, Chain, Addr, - MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI, - Ins[i].PartOffset)); + 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)); + } } } } diff --git a/test/CodeGen/X86/arg-copy-elide.ll b/test/CodeGen/X86/arg-copy-elide.ll index 15afb56ed69..939c2f03e2b 100644 --- a/test/CodeGen/X86/arg-copy-elide.ll +++ b/test/CodeGen/X86/arg-copy-elide.ll @@ -1,5 +1,6 @@ ; RUN: llc -mtriple=i686-windows < %s | FileCheck %s +declare void @addrof_i1(i1*) declare void @addrof_i32(i32*) declare void @addrof_i64(i64*) declare void @addrof_i128(i128*) @@ -41,6 +42,7 @@ entry: ; CHECK: popl %[[csr]] ; CHECK: retl +; We won't copy elide for types needing legalization such as i64 or i1. define i64 @split_i64(i64 %x) { entry: @@ -58,8 +60,10 @@ entry: ; CHECK: andl $-8, %esp ; CHECK-DAG: movl 8(%ebp), %[[csr1]] ; CHECK-DAG: movl 12(%ebp), %[[csr2]] -; CHECK-DAG: leal 8(%ebp), %[[reg:[^ ]*]] -; CHECK: pushl %[[reg]] +; CHECK: movl %edi, 4(%esp) +; CHECK: movl %esi, (%esp) +; CEHCK: movl %esp, %eax +; CHECK: pushl %eax ; CHECK: calll _addrof_i64 ; CHECK-DAG: movl %[[csr1]], %eax ; CHECK-DAG: movl %[[csr2]], %edx @@ -69,6 +73,23 @@ entry: ; CHECK: popl %ebp ; CHECK: retl +define i1 @i1_arg(i1 %x) { + %x.addr = alloca i1 + store i1 %x, i1* %x.addr + call void @addrof_i1(i1* %x.addr) + ret i1 %x +} + +; CHECK-LABEL: _i1_arg: +; CHECK: pushl %ebx +; CHECK: movb 8(%esp), %bl +; CHECK: leal 8(%esp), %eax +; CHECK: pushl %eax +; CHECK: calll _addrof_i1 +; CHECK: addl $4, %esp +; CHECK: movl %ebx, %eax +; CHECK: popl %ebx +; CHECK: retl ; We can't copy elide when an i64 is split between registers and memory in a ; fastcc function.