diff options
author | Antonio Maiorano <amaiorano@google.com> | 2020-02-26 14:23:09 -0500 |
---|---|---|
committer | Antonio Maiorano <amaiorano@google.com> | 2020-02-27 16:20:20 +0000 |
commit | ad3e42a439e2963654d47288cdf407134df4b442 (patch) | |
tree | 77f20c7d94603a206486c823ec1926e25f8da511 | |
parent | 4b34ee3de53138c7d924d6b0c3a36c2a3ebf4bfa (diff) | |
download | swiftshader-ad3e42a439e2963654d47288cdf407134df4b442.tar.gz |
Subzero: add support for variadic calls (System V)
For the System V ABI, variadic calls must store the number of floating
point arguments into RAX. This was mostly working by accident for
our calls to printf since RAX is used as a scratch register, and was
often non-zero, which is all that's really needed for printf with float
args to work; however, when RAX would become 0, printf would print the
wrong thing.
Bug: b/149913889
Change-Id: Id4b519c5416927d537fca078109c0dc850f08359
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/41668
Tested-by: Antonio Maiorano <amaiorano@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
-rw-r--r-- | src/Reactor/SubzeroReactor.cpp | 8 | ||||
-rw-r--r-- | third_party/subzero/src/IceInst.h | 15 | ||||
-rw-r--r-- | third_party/subzero/src/IceTargetLoweringX8632.cpp | 8 | ||||
-rw-r--r-- | third_party/subzero/src/IceTargetLoweringX8632.h | 3 | ||||
-rw-r--r-- | third_party/subzero/src/IceTargetLoweringX8664.cpp | 46 | ||||
-rw-r--r-- | third_party/subzero/src/IceTargetLoweringX8664.h | 3 | ||||
-rw-r--r-- | third_party/subzero/src/IceTargetLoweringX86Base.h | 3 | ||||
-rw-r--r-- | third_party/subzero/src/IceTargetLoweringX86BaseImpl.h | 3 |
8 files changed, 65 insertions, 24 deletions
diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp index e3d8e11cb..bc01e01d0 100644 --- a/src/Reactor/SubzeroReactor.cpp +++ b/src/Reactor/SubzeroReactor.cpp @@ -145,7 +145,7 @@ Ice::Constant *getConstantPointer(Ice::GlobalContext *context, void const *ptr) } // Wrapper for calls on C functions with Ice types -Ice::Variable *Call(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Type retTy, void const *fptr, const std::vector<Ice::Operand *> &iceArgs) +Ice::Variable *Call(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Type retTy, void const *fptr, const std::vector<Ice::Operand *> &iceArgs, bool isVariadic) { // Subzero doesn't support boolean return values. Replace with an i32. if(retTy == Ice::IceType_i1) @@ -159,7 +159,7 @@ Ice::Variable *Call(Ice::Cfg *function, Ice::CfgNode *basicBlock, Ice::Type retT ret = function->makeVariable(retTy); } - auto call = Ice::InstCall::create(function, iceArgs.size(), ret, getConstantPointer(function->getContext(), fptr), false); + auto call = Ice::InstCall::create(function, iceArgs.size(), ret, getConstantPointer(function->getContext(), fptr), false, false, isVariadic); for(auto arg : iceArgs) { call->addArg(arg); @@ -175,7 +175,7 @@ Ice::Variable *Call(Ice::Cfg *function, Ice::CfgNode *basicBlock, Return(fptr)(C { Ice::Type retTy = T(rr::CToReactorT<Return>::getType()); std::vector<Ice::Operand *> iceArgs{ std::forward<RArgs>(args)... }; - return Call(function, basicBlock, retTy, reinterpret_cast<void const *>(fptr), iceArgs); + return Call(function, basicBlock, retTy, reinterpret_cast<void const *>(fptr), iceArgs, false); } // Returns a non-const variable copy of const v @@ -818,7 +818,7 @@ private: #ifdef ENABLE_RR_PRINT void VPrintf(const std::vector<Value *> &vals) { - sz::Call(::function, ::basicBlock, Ice::IceType_i32, reinterpret_cast<const void *>(::printf), V(vals)); + sz::Call(::function, ::basicBlock, Ice::IceType_i32, reinterpret_cast<const void *>(::printf), V(vals), true); } #endif // ENABLE_RR_PRINT diff --git a/third_party/subzero/src/IceInst.h b/third_party/subzero/src/IceInst.h index 8ffb1d2c0..f1caeb495 100644 --- a/third_party/subzero/src/IceInst.h +++ b/third_party/subzero/src/IceInst.h @@ -426,7 +426,8 @@ class InstCall : public InstHighLevel { public: static InstCall *create(Cfg *Func, SizeT NumArgs, Variable *Dest, Operand *CallTarget, bool HasTailCall, - bool IsTargetHelperCall = false) { + bool IsTargetHelperCall = false, + bool IsVariadic = false) { /// Set HasSideEffects to true so that the call instruction can't be /// dead-code eliminated. IntrinsicCalls can override this if the particular /// intrinsic is deletable and has no side-effects. @@ -434,7 +435,7 @@ public: constexpr InstKind Kind = Inst::Call; return new (Func->allocate<InstCall>()) InstCall(Func, NumArgs, Dest, CallTarget, HasTailCall, - IsTargetHelperCall, HasSideEffects, Kind); + IsTargetHelperCall, IsVariadic, HasSideEffects, Kind); } void addArg(Operand *Arg) { addSource(Arg); } Operand *getCallTarget() const { return getSrc(0); } @@ -442,6 +443,7 @@ public: SizeT getNumArgs() const { return getSrcSize() - 1; } bool isTailcall() const { return HasTailCall; } bool isTargetHelperCall() const { return IsTargetHelperCall; } + bool isVariadic() const { return IsVariadic; } bool isMemoryWrite() const override { return true; } void dump(const Cfg *Func) const override; static bool classof(const Inst *Instr) { return Instr->getKind() == Call; } @@ -449,10 +451,10 @@ public: protected: InstCall(Cfg *Func, SizeT NumArgs, Variable *Dest, Operand *CallTarget, - bool HasTailCall, bool IsTargetHelperCall, bool HasSideEff, - InstKind Kind) + bool HasTailCall, bool IsTargetHelperCall, bool IsVariadic, + bool HasSideEff, InstKind Kind) : InstHighLevel(Func, Kind, NumArgs + 1, Dest), HasTailCall(HasTailCall), - IsTargetHelperCall(IsTargetHelperCall) { + IsTargetHelperCall(IsTargetHelperCall), IsVariadic(IsVariadic) { HasSideEffects = HasSideEff; addSource(CallTarget); } @@ -460,6 +462,7 @@ protected: private: const bool HasTailCall; const bool IsTargetHelperCall; + const bool IsVariadic; }; /// Cast instruction (a.k.a. conversion operation). @@ -633,7 +636,7 @@ public: private: InstIntrinsicCall(Cfg *Func, SizeT NumArgs, Variable *Dest, Operand *CallTarget, const Intrinsics::IntrinsicInfo &Info) - : InstCall(Func, NumArgs, Dest, CallTarget, false, false, + : InstCall(Func, NumArgs, Dest, CallTarget, false, false, false, Info.HasSideEffects, Inst::IntrinsicCall), Info(Info) {} diff --git a/third_party/subzero/src/IceTargetLoweringX8632.cpp b/third_party/subzero/src/IceTargetLoweringX8632.cpp index 3262279aa..4a1482568 100644 --- a/third_party/subzero/src/IceTargetLoweringX8632.cpp +++ b/third_party/subzero/src/IceTargetLoweringX8632.cpp @@ -351,7 +351,13 @@ bool TargetX8632::legalizeOptAddrForSandbox(OptAddr *Addr) { return false; } -Inst *TargetX8632::emitCallToTarget(Operand *CallTarget, Variable *ReturnReg) { +Inst *TargetX8632::emitCallToTarget(Operand *CallTarget, Variable *ReturnReg, + size_t NumVariadicFpArgs) { + (void)NumVariadicFpArgs; + // Note that NumVariadicFpArgs is only used for System V x86-64 variadic + // calls, because floating point arguments are passed via vector registers, + // whereas for x86-32, all args are passed via the stack. + std::unique_ptr<AutoBundle> Bundle; if (NeedSandboxing) { if (llvm::isa<Constant>(CallTarget)) { diff --git a/third_party/subzero/src/IceTargetLoweringX8632.h b/third_party/subzero/src/IceTargetLoweringX8632.h index 349fb9208..d712d6b7b 100644 --- a/third_party/subzero/src/IceTargetLoweringX8632.h +++ b/third_party/subzero/src/IceTargetLoweringX8632.h @@ -62,7 +62,8 @@ protected: void emitStackProbe(size_t StackSizeBytes); void lowerIndirectJump(Variable *JumpTarget); void emitGetIP(CfgNode *Node); - Inst *emitCallToTarget(Operand *CallTarget, Variable *ReturnReg) override; + Inst *emitCallToTarget(Operand *CallTarget, Variable *ReturnReg, + size_t NumVariadicFpArgs = 0) override; Variable *moveReturnValueToRegister(Operand *Value, Type ReturnType) override; private: diff --git a/third_party/subzero/src/IceTargetLoweringX8664.cpp b/third_party/subzero/src/IceTargetLoweringX8664.cpp index 5ec9e340c..a4cf854c3 100644 --- a/third_party/subzero/src/IceTargetLoweringX8664.cpp +++ b/third_party/subzero/src/IceTargetLoweringX8664.cpp @@ -640,7 +640,8 @@ void TargetX8664::lowerIndirectJump(Variable *JumpTarget) { _jmp(JumpTarget); } -Inst *TargetX8664::emitCallToTarget(Operand *CallTarget, Variable *ReturnReg) { +Inst *TargetX8664::emitCallToTarget(Operand *CallTarget, Variable *ReturnReg, + size_t NumVariadicFpArgs) { Inst *NewCall = nullptr; auto *CallTargetR = llvm::dyn_cast<Variable>(CallTarget); if (NeedSandboxing) { @@ -700,7 +701,6 @@ Inst *TargetX8664::emitCallToTarget(Operand *CallTarget, Variable *ReturnReg) { _add(T64, r15); CallTarget = T64; } - NewCall = Context.insert<Traits::Insts::Jmp>(CallTarget); } if (ReturnReg != nullptr) { @@ -715,14 +715,42 @@ Inst *TargetX8664::emitCallToTarget(Operand *CallTarget, Variable *ReturnReg) { Variable *T = makeReg(IceType_i64); _movzx(T, CallTargetR); CallTarget = T; - } else if (llvm::isa<Constant>(CallTarget) && - CallTarget->getType() == IceType_i64) { - // x86-64 does not support 64-bit direct calls, so write the value - // to a register and make an indirect call. - Variable *T = makeReg(IceType_i64); - _mov(T, CallTarget); - CallTarget = T; + + } else if (CallTarget->getType() == IceType_i64) { + // x86-64 does not support 64-bit direct calls, so write the value to a + // register and make an indirect call for Constant call targets. + RegNumT TargetReg = {}; + + // System V: force r11 when calling a variadic function so that rax isn't + // used, since rax stores the number of FP args (see NumVariadicFpArgs + // usage below). +#if !defined(SUBZERO_USE_MICROSOFT_ABI) + if (NumVariadicFpArgs > 0) + TargetReg = Traits::RegisterSet::Reg_r11; +#endif + + if (llvm::isa<Constant>(CallTarget)) { + Variable *T = makeReg(IceType_i64, TargetReg); + _mov(T, CallTarget); + CallTarget = T; + } else if (llvm::isa<Variable>(CallTarget)) { + Operand *T = legalizeToReg(CallTarget, TargetReg); + CallTarget = T; + } + } + + // System V: store number of FP args in RAX for variadic calls +#if !defined(SUBZERO_USE_MICROSOFT_ABI) + if (NumVariadicFpArgs > 0) { + // Store number of FP args (stored in XMM registers) in RAX for variadic + // calls + auto *NumFpArgs = Ctx->getConstantInt64(NumVariadicFpArgs); + Variable *NumFpArgsReg = + legalizeToReg(NumFpArgs, Traits::RegisterSet::Reg_rax); + Context.insert<InstFakeUse>(NumFpArgsReg); } +#endif + NewCall = Context.insert<Traits::Insts::Call>(ReturnReg, CallTarget); } return NewCall; diff --git a/third_party/subzero/src/IceTargetLoweringX8664.h b/third_party/subzero/src/IceTargetLoweringX8664.h index 3f330501a..e412fa139 100644 --- a/third_party/subzero/src/IceTargetLoweringX8664.h +++ b/third_party/subzero/src/IceTargetLoweringX8664.h @@ -65,7 +65,8 @@ protected: void emitStackProbe(size_t StackSizeBytes); void lowerIndirectJump(Variable *JumpTarget); void emitGetIP(CfgNode *Node); - Inst *emitCallToTarget(Operand *CallTarget, Variable *ReturnReg) override; + Inst *emitCallToTarget(Operand *CallTarget, Variable *ReturnReg, + size_t NumVariadicFpArgs = 0) override; Variable *moveReturnValueToRegister(Operand *Value, Type ReturnType) override; private: diff --git a/third_party/subzero/src/IceTargetLoweringX86Base.h b/third_party/subzero/src/IceTargetLoweringX86Base.h index 46df7be05..f4019c622 100644 --- a/third_party/subzero/src/IceTargetLoweringX86Base.h +++ b/third_party/subzero/src/IceTargetLoweringX86Base.h @@ -384,7 +384,8 @@ protected: /// Emit just the call instruction (without argument or return variable /// processing), sandboxing if needed. - virtual Inst *emitCallToTarget(Operand *CallTarget, Variable *ReturnReg) = 0; + virtual Inst *emitCallToTarget(Operand *CallTarget, Variable *ReturnReg, + size_t NumVariadicFpArgs = 0) = 0; /// Materialize the moves needed to return a value of the specified type. virtual Variable *moveReturnValueToRegister(Operand *Value, Type ReturnType) = 0; diff --git a/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h b/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h index 5b19e7cc6..41173f509 100644 --- a/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h +++ b/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h @@ -2816,7 +2816,8 @@ void TargetX86Base<TraitsType>::lowerCall(const InstCall *Instr) { // Emit the call to the function. Operand *CallTarget = legalize(Instr->getCallTarget(), Legal_Reg | Legal_Imm | Legal_AddrAbs); - Inst *NewCall = emitCallToTarget(CallTarget, ReturnReg); + size_t NumVariadicFpArgs = Instr->isVariadic() ? XmmArgs.size() : 0; + Inst *NewCall = emitCallToTarget(CallTarget, ReturnReg, NumVariadicFpArgs); // Keep the upper return register live on 32-bit platform. if (ReturnRegHi) Context.insert<InstFakeDef>(ReturnRegHi); |