From 244264d050259a17c5eb4cceb9c5878d5e714a4e Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Tue, 17 Jan 2017 22:30:10 +0000 Subject: [PATCH] GlobalISel: correctly handle varargs Some platforms (notably iOS) use a different calling convention for unnamed vs named parameters in varargs functions, so we need to keep track of this information when translating calls. Since not many platforms are involved, the guts of the special handling is in the ValueHandler class (with a generic implementation that should work for most targets). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@292283 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../llvm/CodeGen/GlobalISel/CallLowering.h | 24 +++++--- lib/CodeGen/GlobalISel/CallLowering.cpp | 7 ++- lib/Target/AArch64/AArch64CallLowering.cpp | 55 ++++++++++++------- lib/Target/ARM/ARMCallLowering.cpp | 18 +++--- .../GlobalISel/arm64-callingconv-ios.ll | 28 ++++++++++ .../AArch64/GlobalISel/arm64-callingconv.ll | 24 ++++++++ 6 files changed, 119 insertions(+), 37 deletions(-) create mode 100644 test/CodeGen/AArch64/GlobalISel/arm64-callingconv-ios.ll diff --git a/include/llvm/CodeGen/GlobalISel/CallLowering.h b/include/llvm/CodeGen/GlobalISel/CallLowering.h index 0b157bf937a..8eec32cece2 100644 --- a/include/llvm/CodeGen/GlobalISel/CallLowering.h +++ b/include/llvm/CodeGen/GlobalISel/CallLowering.h @@ -35,9 +35,11 @@ public: unsigned Reg; Type *Ty; ISD::ArgFlagsTy Flags; + bool IsFixed; - ArgInfo(unsigned Reg, Type *Ty, ISD::ArgFlagsTy Flags = ISD::ArgFlagsTy{}) - : Reg(Reg), Ty(Ty), Flags(Flags) {} + ArgInfo(unsigned Reg, Type *Ty, ISD::ArgFlagsTy Flags = ISD::ArgFlagsTy{}, + bool IsFixed = true) + : Reg(Reg), Ty(Ty), Flags(Flags), IsFixed(IsFixed) {} }; /// Argument handling is mostly uniform between the four places that @@ -70,13 +72,21 @@ public: unsigned extendRegister(unsigned ValReg, CCValAssign &VA); - ValueHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI) - : MIRBuilder(MIRBuilder), MRI(MRI) {} + virtual bool assignArg(unsigned ValNo, MVT ValVT, MVT LocVT, + CCValAssign::LocInfo LocInfo, const ArgInfo &Info, + CCState &State) { + return AssignFn(ValNo, ValVT, LocVT, LocInfo, Info.Flags, State); + } + + ValueHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI, + CCAssignFn *AssignFn) + : MIRBuilder(MIRBuilder), MRI(MRI), AssignFn(AssignFn) {} virtual ~ValueHandler() {} MachineIRBuilder &MIRBuilder; MachineRegisterInfo &MRI; + CCAssignFn *AssignFn; }; protected: @@ -96,12 +106,12 @@ protected: void setArgFlags(ArgInfo &Arg, unsigned OpNum, const DataLayout &DL, const FuncInfoTy &FuncInfo) const; - /// Invoke the \p AssignFn on each of the given \p Args and then use + /// Invoke Handler::assignArg on each of the given \p Args and then use /// \p Callback to move them to the assigned locations. /// /// \return True if everything has succeeded, false otherwise. - bool handleAssignments(MachineIRBuilder &MIRBuilder, CCAssignFn *AssignFn, - ArrayRef Args, ValueHandler &Callback) const; + bool handleAssignments(MachineIRBuilder &MIRBuilder, ArrayRef Args, + ValueHandler &Callback) const; public: CallLowering(const TargetLowering *TLI) : TLI(TLI) {} diff --git a/lib/CodeGen/GlobalISel/CallLowering.cpp b/lib/CodeGen/GlobalISel/CallLowering.cpp index 13212212fa0..33e70856c95 100644 --- a/lib/CodeGen/GlobalISel/CallLowering.cpp +++ b/lib/CodeGen/GlobalISel/CallLowering.cpp @@ -33,8 +33,10 @@ bool CallLowering::lowerCall( // we'll pass to the assigner function. SmallVector OrigArgs; unsigned i = 0; + unsigned NumFixedArgs = CI.getFunctionType()->getNumParams(); for (auto &Arg : CI.arg_operands()) { - ArgInfo OrigArg{ArgRegs[i], Arg->getType(), ISD::ArgFlagsTy{}}; + ArgInfo OrigArg{ArgRegs[i], Arg->getType(), ISD::ArgFlagsTy{}, + i < NumFixedArgs}; setArgFlags(OrigArg, i + 1, DL, CI); OrigArgs.push_back(OrigArg); ++i; @@ -103,7 +105,6 @@ CallLowering::setArgFlags(CallLowering::ArgInfo &Arg, unsigned OpIdx, const CallInst &FuncInfo) const; bool CallLowering::handleAssignments(MachineIRBuilder &MIRBuilder, - CCAssignFn *AssignFn, ArrayRef Args, ValueHandler &Handler) const { MachineFunction &MF = MIRBuilder.getMF(); @@ -116,7 +117,7 @@ bool CallLowering::handleAssignments(MachineIRBuilder &MIRBuilder, unsigned NumArgs = Args.size(); for (unsigned i = 0; i != NumArgs; ++i) { MVT CurVT = MVT::getVT(Args[i].Ty); - if (AssignFn(i, CurVT, CurVT, CCValAssign::Full, Args[i].Flags, CCInfo)) + if (Handler.assignArg(i, CurVT, CurVT, CCValAssign::Full, Args[i], CCInfo)) return false; } diff --git a/lib/Target/AArch64/AArch64CallLowering.cpp b/lib/Target/AArch64/AArch64CallLowering.cpp index cd8ec784613..7efee8bf8c2 100644 --- a/lib/Target/AArch64/AArch64CallLowering.cpp +++ b/lib/Target/AArch64/AArch64CallLowering.cpp @@ -35,8 +35,9 @@ AArch64CallLowering::AArch64CallLowering(const AArch64TargetLowering &TLI) } struct IncomingArgHandler : public CallLowering::ValueHandler { - IncomingArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI) - : ValueHandler(MIRBuilder, MRI) {} + IncomingArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI, + CCAssignFn *AssignFn) + : ValueHandler(MIRBuilder, MRI, AssignFn) {} unsigned getStackAddress(uint64_t Size, int64_t Offset, MachinePointerInfo &MPO) override { @@ -70,8 +71,9 @@ struct IncomingArgHandler : public CallLowering::ValueHandler { }; struct FormalArgHandler : public IncomingArgHandler { - FormalArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI) - : IncomingArgHandler(MIRBuilder, MRI) {} + FormalArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI, + CCAssignFn *AssignFn) + : IncomingArgHandler(MIRBuilder, MRI, AssignFn) {} void markPhysRegUsed(unsigned PhysReg) override { MIRBuilder.getMBB().addLiveIn(PhysReg); @@ -80,8 +82,8 @@ struct FormalArgHandler : public IncomingArgHandler { struct CallReturnHandler : public IncomingArgHandler { CallReturnHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI, - MachineInstrBuilder MIB) - : IncomingArgHandler(MIRBuilder, MRI), MIB(MIB) {} + MachineInstrBuilder MIB, CCAssignFn *AssignFn) + : IncomingArgHandler(MIRBuilder, MRI, AssignFn), MIB(MIB) {} void markPhysRegUsed(unsigned PhysReg) override { MIB.addDef(PhysReg, RegState::Implicit); @@ -92,8 +94,10 @@ struct CallReturnHandler : public IncomingArgHandler { struct OutgoingArgHandler : public CallLowering::ValueHandler { OutgoingArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI, - MachineInstrBuilder MIB) - : ValueHandler(MIRBuilder, MRI), MIB(MIB) {} + MachineInstrBuilder MIB, CCAssignFn *AssignFn, + CCAssignFn *AssignFnVarArg) + : ValueHandler(MIRBuilder, MRI, AssignFn), MIB(MIB), + AssignFnVarArg(AssignFnVarArg) {} unsigned getStackAddress(uint64_t Size, int64_t Offset, MachinePointerInfo &MPO) override { @@ -126,7 +130,17 @@ struct OutgoingArgHandler : public CallLowering::ValueHandler { MIRBuilder.buildStore(ValVReg, Addr, *MMO); } + virtual bool assignArg(unsigned ValNo, MVT ValVT, MVT LocVT, + CCValAssign::LocInfo LocInfo, + const CallLowering::ArgInfo &Info, + CCState &State) override { + if (Info.IsFixed) + return AssignFn(ValNo, ValVT, LocVT, LocInfo, Info.Flags, State); + return AssignFnVarArg(ValNo, ValVT, LocVT, LocInfo, Info.Flags, State); + } + MachineInstrBuilder MIB; + CCAssignFn *AssignFnVarArg; }; void AArch64CallLowering::splitToValueTypes( @@ -144,7 +158,7 @@ void AArch64CallLowering::splitToValueTypes( // No splitting to do, but we want to replace the original type (e.g. [1 x // double] -> double). SplitArgs.emplace_back(OrigArg.Reg, SplitVTs[0].getTypeForEVT(Ctx), - OrigArg.Flags); + OrigArg.Flags, OrigArg.IsFixed); return; } @@ -154,7 +168,7 @@ void AArch64CallLowering::splitToValueTypes( Type *SplitTy = SplitVT.getTypeForEVT(Ctx); SplitArgs.push_back( ArgInfo{MRI.createGenericVirtualRegister(LLT{*SplitTy, DL}), SplitTy, - OrigArg.Flags}); + OrigArg.Flags, OrigArg.IsFixed}); } SmallVector BitOffsets; @@ -191,8 +205,8 @@ bool AArch64CallLowering::lowerReturn(MachineIRBuilder &MIRBuilder, MIRBuilder.buildExtract(Regs, Offsets, VReg); }); - OutgoingArgHandler Handler(MIRBuilder, MRI, MIB); - Success = handleAssignments(MIRBuilder, AssignFn, SplitArgs, Handler); + OutgoingArgHandler Handler(MIRBuilder, MRI, MIB, AssignFn, AssignFn); + Success = handleAssignments(MIRBuilder, SplitArgs, Handler); } MIRBuilder.insertInstr(MIB); @@ -227,8 +241,8 @@ bool AArch64CallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder, CCAssignFn *AssignFn = TLI.CCAssignFnForCall(F.getCallingConv(), /*IsVarArg=*/false); - FormalArgHandler Handler(MIRBuilder, MRI); - if (!handleAssignments(MIRBuilder, AssignFn, SplitArgs, Handler)) + FormalArgHandler Handler(MIRBuilder, MRI, AssignFn); + if (!handleAssignments(MIRBuilder, SplitArgs, Handler)) return false; // Move back to the end of the basic block. @@ -256,8 +270,10 @@ bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, // Find out which ABI gets to decide where things go. const AArch64TargetLowering &TLI = *getTLI(); - CCAssignFn *CallAssignFn = + CCAssignFn *AssignFnFixed = TLI.CCAssignFnForCall(F.getCallingConv(), /*IsVarArg=*/false); + CCAssignFn *AssignFnVarArg = + TLI.CCAssignFnForCall(F.getCallingConv(), /*IsVarArg=*/true); // Create a temporarily-floating call instruction so we can add the implicit // uses of arg registers. @@ -271,8 +287,9 @@ bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, // Do the actual argument marshalling. SmallVector PhysRegs; - OutgoingArgHandler Handler(MIRBuilder, MRI, MIB); - if (!handleAssignments(MIRBuilder, CallAssignFn, SplitArgs, Handler)) + OutgoingArgHandler Handler(MIRBuilder, MRI, MIB, AssignFnFixed, + AssignFnVarArg); + if (!handleAssignments(MIRBuilder, SplitArgs, Handler)) return false; // Now we can add the actual call instruction to the correct basic block. @@ -304,8 +321,8 @@ bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, std::back_inserter(SplitRegs)); }); - CallReturnHandler Handler(MIRBuilder, MRI, MIB); - if (!handleAssignments(MIRBuilder, RetAssignFn, SplitArgs, Handler)) + CallReturnHandler Handler(MIRBuilder, MRI, MIB, RetAssignFn); + if (!handleAssignments(MIRBuilder, SplitArgs, Handler)) return false; if (!RegOffsets.empty()) diff --git a/lib/Target/ARM/ARMCallLowering.cpp b/lib/Target/ARM/ARMCallLowering.cpp index 4b5fa4bb8c5..32b823f941d 100644 --- a/lib/Target/ARM/ARMCallLowering.cpp +++ b/lib/Target/ARM/ARMCallLowering.cpp @@ -43,8 +43,8 @@ static bool isSupportedType(const DataLayout &DL, const ARMTargetLowering &TLI, namespace { struct FuncReturnHandler : public CallLowering::ValueHandler { FuncReturnHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI, - MachineInstrBuilder &MIB) - : ValueHandler(MIRBuilder, MRI), MIB(MIB) {} + MachineInstrBuilder &MIB, CCAssignFn *AssignFn) + : ValueHandler(MIRBuilder, MRI, AssignFn), MIB(MIB) {} unsigned getStackAddress(uint64_t Size, int64_t Offset, MachinePointerInfo &MPO) override { @@ -99,8 +99,8 @@ bool ARMCallLowering::lowerReturnVal(MachineIRBuilder &MIRBuilder, ArgInfo RetInfo(VReg, Val->getType()); setArgFlags(RetInfo, AttributeSet::ReturnIndex, DL, F); - FuncReturnHandler RetHandler(MIRBuilder, MF.getRegInfo(), Ret); - return handleAssignments(MIRBuilder, AssignFn, RetInfo, RetHandler); + FuncReturnHandler RetHandler(MIRBuilder, MF.getRegInfo(), Ret, AssignFn); + return handleAssignments(MIRBuilder, RetInfo, RetHandler); } bool ARMCallLowering::lowerReturn(MachineIRBuilder &MIRBuilder, @@ -118,8 +118,9 @@ bool ARMCallLowering::lowerReturn(MachineIRBuilder &MIRBuilder, namespace { struct FormalArgHandler : public CallLowering::ValueHandler { - FormalArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI) - : ValueHandler(MIRBuilder, MRI) {} + FormalArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI, + CCAssignFn AssignFn) + : ValueHandler(MIRBuilder, MRI, AssignFn) {} unsigned getStackAddress(uint64_t Size, int64_t Offset, MachinePointerInfo &MPO) override { @@ -198,6 +199,7 @@ bool ARMCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder, Idx++; } - FormalArgHandler ArgHandler(MIRBuilder, MIRBuilder.getMF().getRegInfo()); - return handleAssignments(MIRBuilder, AssignFn, ArgInfos, ArgHandler); + FormalArgHandler ArgHandler(MIRBuilder, MIRBuilder.getMF().getRegInfo(), + AssignFn); + return handleAssignments(MIRBuilder, ArgInfos, ArgHandler); } diff --git a/test/CodeGen/AArch64/GlobalISel/arm64-callingconv-ios.ll b/test/CodeGen/AArch64/GlobalISel/arm64-callingconv-ios.ll new file mode 100644 index 00000000000..a70cee0efcb --- /dev/null +++ b/test/CodeGen/AArch64/GlobalISel/arm64-callingconv-ios.ll @@ -0,0 +1,28 @@ +; RUN: llc -O0 -stop-after=irtranslator -global-isel -verify-machineinstrs %s -o - 2>&1 | FileCheck %s + +target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128" +target triple = "aarch64-apple-ios9.0" + +; CHECK-LABEL: name: test_varargs +; CHECK: [[ANSWER:%[0-9]+]](s32) = G_CONSTANT i32 42 +; CHECK: [[D_ONE:%[0-9]+]](s64) = G_FCONSTANT double 1.000000e+00 +; CHECK: [[TWELVE:%[0-9]+]](s64) = G_CONSTANT i64 12 +; CHECK: [[THREE:%[0-9]+]](s8) = G_CONSTANT i8 3 +; CHECK: [[ONE:%[0-9]+]](s16) = G_CONSTANT i16 1 +; CHECK: [[FOUR:%[0-9]+]](s32) = G_CONSTANT i32 4 +; CHECK: [[F_ONE:%[0-9]+]](s32) = G_FCONSTANT float 1.000000e+00 +; CHECK: [[TWO:%[0-9]+]](s64) = G_FCONSTANT double 2.000000e+00 + +; CHECK: %w0 = COPY [[ANSWER]] +; CHECK: %d0 = COPY [[D_ONE]] +; CHECK: %x1 = COPY [[TWELVE]] +; CHECK: G_STORE [[THREE]](s8), {{%[0-9]+}}(p0) :: (store 1 into stack, align 0) +; CHECK: G_STORE [[ONE]](s16), {{%[0-9]+}}(p0) :: (store 2 into stack + 8, align 0) +; CHECK: G_STORE [[FOUR]](s32), {{%[0-9]+}}(p0) :: (store 4 into stack + 16, align 0) +; CHECK: G_STORE [[F_ONE]](s32), {{%[0-9]+}}(p0) :: (store 4 into stack + 24, align 0) +; CHECK: G_STORE [[TWO]](s64), {{%[0-9]+}}(p0) :: (store 8 into stack + 32, align 0) +declare void @varargs(i32, double, i64, ...) +define void @test_varargs() { + call void(i32, double, i64, ...) @varargs(i32 42, double 1.0, i64 12, i8 3, i16 1, i32 4, float 1.0, double 2.0) + ret void +} diff --git a/test/CodeGen/AArch64/GlobalISel/arm64-callingconv.ll b/test/CodeGen/AArch64/GlobalISel/arm64-callingconv.ll index 95b2ea2b4ff..350ae0e4c08 100644 --- a/test/CodeGen/AArch64/GlobalISel/arm64-callingconv.ll +++ b/test/CodeGen/AArch64/GlobalISel/arm64-callingconv.ll @@ -56,3 +56,27 @@ define i8* @args_ptrs(i8* %x0, i16* %x1, <2 x i8>* %x2, {i8, i16, i32}* %x3, define [1 x double] @args_arr([1 x double] %d0) { ret [1 x double] %d0 } + +; CHECK-LABEL: name: test_varargs +; CHECK: [[ANSWER:%[0-9]+]](s32) = G_CONSTANT i32 42 +; CHECK: [[D_ONE:%[0-9]+]](s64) = G_FCONSTANT double 1.000000e+00 +; CHECK: [[TWELVE:%[0-9]+]](s64) = G_CONSTANT i64 12 +; CHECK: [[THREE:%[0-9]+]](s8) = G_CONSTANT i8 3 +; CHECK: [[ONE:%[0-9]+]](s16) = G_CONSTANT i16 1 +; CHECK: [[FOUR:%[0-9]+]](s32) = G_CONSTANT i32 4 +; CHECK: [[F_ONE:%[0-9]+]](s32) = G_FCONSTANT float 1.000000e+00 +; CHECK: [[TWO:%[0-9]+]](s64) = G_FCONSTANT double 2.000000e+00 + +; CHECK: %w0 = COPY [[ANSWER]] +; CHECK: %d0 = COPY [[D_ONE]] +; CHECK: %x1 = COPY [[TWELVE]] +; CHECK: %w2 = COPY [[THREE]](s8) +; CHECK: %w3 = COPY [[ONE]](s16) +; CHECK: %w4 = COPY [[FOUR]](s32) +; CHECK: %s1 = COPY [[F_ONE]](s32) +; CHECK: %d2 = COPY [[TWO]](s64) +declare void @varargs(i32, double, i64, ...) +define void @test_varargs() { + call void(i32, double, i64, ...) @varargs(i32 42, double 1.0, i64 12, i8 3, i16 1, i32 4, float 1.0, double 2.0) + ret void +} -- 2.50.1