Index: include/llvm/IR/Function.h =================================================================== --- include/llvm/IR/Function.h +++ include/llvm/IR/Function.h @@ -297,7 +297,8 @@ /// @brief Determine if the function returns a structure through first /// pointer argument. bool hasStructRetAttr() const { - return AttributeSets.hasAttribute(1, Attribute::StructRet); + return AttributeSets.hasAttribute(1, Attribute::StructRet) || + AttributeSets.hasAttribute(2, Attribute::StructRet); } /// @brief Determine if the parameter does not alias other parameters. Index: lib/IR/Function.cpp =================================================================== --- lib/IR/Function.cpp +++ lib/IR/Function.cpp @@ -207,10 +207,10 @@ // Function Implementation //===----------------------------------------------------------------------===// -Function::Function(FunctionType *Ty, LinkageTypes Linkage, const Twine &name, - Module *ParentModule) - : GlobalValue(PointerType::getUnqual(Ty), Value::FunctionVal, nullptr, 0, - Linkage, name) { +Function::Function(FunctionType *Ty, LinkageTypes Linkage, + const Twine &name, Module *ParentModule) + : GlobalValue(PointerType::getUnqual(Ty), + Value::FunctionVal, nullptr, 0, Linkage, name) { assert(FunctionType::isValidReturnType(getReturnType()) && "invalid return type"); SymTab = new ValueSymbolTable(); Index: lib/IR/Verifier.cpp =================================================================== --- lib/IR/Verifier.cpp +++ lib/IR/Verifier.cpp @@ -820,6 +820,7 @@ bool SawNest = false; bool SawReturned = false; + bool SawSRet = false; for (unsigned i = 0, e = Attrs.getNumSlots(); i != e; ++i) { unsigned Idx = Attrs.getSlotIndex(i); @@ -850,8 +851,12 @@ SawReturned = true; } - if (Attrs.hasAttribute(Idx, Attribute::StructRet)) - Assert1(Idx == 1, "Attribute sret is not on first parameter!", V); + if (Attrs.hasAttribute(Idx, Attribute::StructRet)) { + Assert1(!SawSRet, "Cannot have multiple 'sret' parameters!", V); + Assert1(Idx == 1 || Idx == 2, + "Attribute 'sret' is not on first or second parameter!", V); + SawSRet = true; + } if (Attrs.hasAttribute(Idx, Attribute::InAlloca)) { Assert1(Idx == FT->getNumParams(), Index: lib/Target/Mips/MipsISelLowering.cpp =================================================================== --- lib/Target/Mips/MipsISelLowering.cpp +++ lib/Target/Mips/MipsISelLowering.cpp @@ -2659,20 +2659,21 @@ InVals.push_back(Load); OutChains.push_back(Load.getValue(1)); } - } - // The mips ABIs for returning structs by value requires that we copy - // the sret argument into $v0 for the return. Save the argument into - // a virtual register so that we can access it from the return points. - if (DAG.getMachineFunction().getFunction()->hasStructRetAttr()) { - unsigned Reg = MipsFI->getSRetReturnReg(); - if (!Reg) { - Reg = MF.getRegInfo().createVirtualRegister( - getRegClassFor(isN64() ? MVT::i64 : MVT::i32)); - MipsFI->setSRetReturnReg(Reg); + // The mips ABIs for returning structs by value requires that we copy + // the sret argument into $v0 for the return. Save the argument into + // a virtual register so that we can access it from the return points. + if (Flags.isSRet()) { + unsigned Reg = MipsFI->getSRetReturnReg(); + if (!Reg) { + Reg = MF.getRegInfo().createVirtualRegister( + getRegClassFor(isN64() ? MVT::i64 : MVT::i32)); + MipsFI->setSRetReturnReg(Reg); + } + SDValue Copy = + DAG.getCopyToReg(DAG.getEntryNode(), DL, Reg, InVals.back()); + Chain = DAG.getNode(ISD::TokenFactor, DL, MVT::Other, Copy, Chain); } - SDValue Copy = DAG.getCopyToReg(DAG.getEntryNode(), DL, Reg, InVals[0]); - Chain = DAG.getNode(ISD::TokenFactor, DL, MVT::Other, Copy, Chain); } if (IsVarArg) Index: lib/Target/Sparc/SparcISelLowering.cpp =================================================================== --- lib/Target/Sparc/SparcISelLowering.cpp +++ lib/Target/Sparc/SparcISelLowering.cpp @@ -358,7 +358,12 @@ for (unsigned i = 0, e = ArgLocs.size(); i != e; ++i) { CCValAssign &VA = ArgLocs[i]; - if (i == 0 && Ins[i].Flags.isSRet()) { + if (Ins[i].Flags.isSRet()) { + if (i != 0) { + report_fatal_error( + "sret only supported for the first parameter on sparc"); + } + // Get SRet from [%fp+64]. int FrameIdx = MF.getFrameInfo()->CreateFixedObject(4, 64, true); SDValue FIPtr = DAG.getFrameIndex(FrameIdx, MVT::i32); Index: lib/Target/X86/X86ISelLowering.cpp =================================================================== --- lib/Target/X86/X86ISelLowering.cpp +++ lib/Target/X86/X86ISelLowering.cpp @@ -2299,24 +2299,24 @@ MachinePointerInfo(), false, false, false, 0); InVals.push_back(ArgValue); - } - // The x86-64 ABIs require that for returning structs by value we copy - // the sret argument into %rax/%eax (depending on ABI) for the return. - // Win32 requires us to put the sret argument to %eax as well. - // Save the argument into a virtual register so that we can access it - // from the return points. - if (MF.getFunction()->hasStructRetAttr() && - (Subtarget->is64Bit() || Subtarget->isTargetKnownWindowsMSVC())) { - X86MachineFunctionInfo *FuncInfo = MF.getInfo(); - unsigned Reg = FuncInfo->getSRetReturnReg(); - if (!Reg) { - MVT PtrTy = getPointerTy(); - Reg = MF.getRegInfo().createVirtualRegister(getRegClassFor(PtrTy)); - FuncInfo->setSRetReturnReg(Reg); + // The x86-64 ABIs require that for returning structs by value we copy + // the sret argument into %rax/%eax (depending on ABI) for the return. + // Win32 requires us to put the sret argument to %eax as well. + // Save the argument into a virtual register so that we can access it + // from the return points. + if (Ins[i].Flags.isSRet() && + (Subtarget->is64Bit() || Subtarget->isTargetKnownWindowsMSVC())) { + unsigned Reg = FuncInfo->getSRetReturnReg(); + if (!Reg) { + MVT PtrTy = getPointerTy(); + Reg = MF.getRegInfo().createVirtualRegister(getRegClassFor(PtrTy)); + FuncInfo->setSRetReturnReg(Reg); + } + SDValue Copy = + DAG.getCopyToReg(DAG.getEntryNode(), dl, Reg, InVals.back()); + Chain = DAG.getNode(ISD::TokenFactor, dl, MVT::Other, Copy, Chain); } - SDValue Copy = DAG.getCopyToReg(DAG.getEntryNode(), dl, Reg, InVals[0]); - Chain = DAG.getNode(ISD::TokenFactor, dl, MVT::Other, Copy, Chain); } unsigned StackSize = CCInfo.getNextStackOffset(); Index: test/CodeGen/Mips/mips64-sret.ll =================================================================== --- test/CodeGen/Mips/mips64-sret.ll +++ test/CodeGen/Mips/mips64-sret.ll @@ -1,16 +1,23 @@ -; RUN: llc -march=mips64el -mcpu=mips64r2 -mattr=n64 -O3 < %s | FileCheck %s +; RUN: llc -march=mips64el -mcpu=mips64r2 -mattr=n64 < %s | FileCheck %s -%struct.S = type { [8 x i32] } +define void @foo(i32* noalias sret %agg.result) nounwind { +entry: +; CHECK-LABEL: foo: +; CHECK: sw {{.*}}, 0($4) +; CHECK: jr $ra +; CHECK-NEXT: move $2, $4 -@g = common global %struct.S zeroinitializer, align 4 + store i32 42, i32* %agg.result + ret void +} -define void @f(%struct.S* noalias sret %agg.result) nounwind { +define void @bar(i32 %v, i32* noalias sret %agg.result) nounwind { entry: -; CHECK: move $2, $4 +; CHECK-LABEL: bar: +; CHECK: sw $4, 0($5) +; CHECK: jr $ra +; CHECK-NEXT: move $2, $5 - %0 = bitcast %struct.S* %agg.result to i8* - call void @llvm.memcpy.p0i8.p0i8.i64(i8* %0, i8* bitcast (%struct.S* @g to i8*), i64 32, i32 4, i1 false) + store i32 %v, i32* %agg.result ret void } - -declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i32, i1) nounwind Index: test/CodeGen/X86/win32_sret.ll =================================================================== --- test/CodeGen/X86/win32_sret.ll +++ test/CodeGen/X86/win32_sret.ll @@ -181,3 +181,63 @@ ret void } declare x86_thiscallcc void @test6_g(%struct.test6* sret, %struct.test6*) + +; Flipping the parameters at the IR level generates the same code. +%struct.test7 = type { i32, i32, i32 } +define void @test7_f(%struct.test7* %x) nounwind { +; WIN32-LABEL: _test7_f: +; MINGW_X86-LABEL: _test7_f: +; CYGWIN-LABEL: _test7_f: +; LINUX-LABEL: test7_f: + +; The %x argument is moved to %ecx on all OSs. It will be the this pointer. +; WIN32: movl 8(%ebp), %ecx +; MINGW_X86: movl 8(%ebp), %ecx +; CYGWIN: movl 8(%ebp), %ecx + +; The sret pointer is (%esp) +; WIN32: leal 8(%esp), %[[REG:e[a-d]x]] +; WIN32-NEXT: movl %[[REG]], (%e{{([a-d]x)|(sp)}}) +; MINGW_X86: leal 8(%esp), %[[REG:e[a-d]x]] +; MINGW_X86-NEXT: movl %[[REG]], (%e{{([a-d]x)|(sp)}}) +; CYGWIN: leal 8(%esp), %[[REG:e[a-d]x]] +; CYGWIN-NEXT: movl %[[REG]], (%e{{([a-d]x)|(sp)}}) + + %tmp = alloca %struct.test7, align 4 + call x86_thiscallcc void @test7_g(%struct.test7* %x, %struct.test7* sret %tmp) + ret void +} + +define x86_thiscallcc void @test7_g(%struct.test7* %in, %struct.test7* sret %out) { + %s = getelementptr %struct.test7* %in, i32 0, i32 0 + %d = getelementptr %struct.test7* %out, i32 0, i32 0 + %v = load i32* %s + store i32 %v, i32* %d + call void @clobber_eax() + ret void + +; Make sure we return the second parameter in %eax. +; WIN32-LABEL: _test7_g: +; WIN32: calll _clobber_eax +; WIN32: movl {{.*}}, %eax +; WIN32: retl +} + +declare void @clobber_eax() + +; Test what happens if the first parameter has to be split by codegen. +; Realistically, no frontend will generate code like this, but here it is for +; completeness. +define void @test8_f(i64 inreg %a, i64* sret %out) { + store i64 %a, i64* %out + call void @clobber_eax() + ret void + +; WIN32-LABEL: _test8_f: +; WIN32: movl {{[0-9]+}}(%esp), %[[out:[a-z]+]] +; WIN32-DAG: movl %edx, 4(%[[out]]) +; WIN32-DAG: movl %eax, (%[[out]]) +; WIN32: calll _clobber_eax +; WIN32: movl {{.*}}, %eax +; WIN32: retl +} Index: test/Verifier/sret.ll =================================================================== --- /dev/null +++ test/Verifier/sret.ll @@ -0,0 +1,7 @@ +; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s + +declare void @a(i32* sret %a, i32* sret %b) +; CHECK: Cannot have multiple 'sret' parameters! + +declare void @b(i32* %a, i32* %b, i32* sret %c) +; CHECK: Attribute 'sret' is not on first or second parameter!