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>
diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp
index e3d8e11..bc01e01 100644
--- a/src/Reactor/SubzeroReactor.cpp
+++ b/src/Reactor/SubzeroReactor.cpp
@@ -145,7 +145,7 @@
}
// 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 @@
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::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 @@
#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 8ffb1d2..f1caeb4 100644
--- a/third_party/subzero/src/IceInst.h
+++ b/third_party/subzero/src/IceInst.h
@@ -426,7 +426,8 @@
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 @@
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 @@
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 @@
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 @@
private:
const bool HasTailCall;
const bool IsTargetHelperCall;
+ const bool IsVariadic;
};
/// Cast instruction (a.k.a. conversion operation).
@@ -633,7 +636,7 @@
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 3262279..4a14825 100644
--- a/third_party/subzero/src/IceTargetLoweringX8632.cpp
+++ b/third_party/subzero/src/IceTargetLoweringX8632.cpp
@@ -351,7 +351,13 @@
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 349fb92..d712d6b 100644
--- a/third_party/subzero/src/IceTargetLoweringX8632.h
+++ b/third_party/subzero/src/IceTargetLoweringX8632.h
@@ -62,7 +62,8 @@
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 5ec9e34..a4cf854 100644
--- a/third_party/subzero/src/IceTargetLoweringX8664.cpp
+++ b/third_party/subzero/src/IceTargetLoweringX8664.cpp
@@ -640,7 +640,8 @@
_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 @@
_add(T64, r15);
CallTarget = T64;
}
-
NewCall = Context.insert<Traits::Insts::Jmp>(CallTarget);
}
if (ReturnReg != nullptr) {
@@ -715,14 +715,42 @@
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 3f33050..e412fa1 100644
--- a/third_party/subzero/src/IceTargetLoweringX8664.h
+++ b/third_party/subzero/src/IceTargetLoweringX8664.h
@@ -65,7 +65,8 @@
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 46df7be..f4019c6 100644
--- a/third_party/subzero/src/IceTargetLoweringX86Base.h
+++ b/third_party/subzero/src/IceTargetLoweringX86Base.h
@@ -384,7 +384,8 @@
/// 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 5b19e7c..41173f5 100644
--- a/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
+++ b/third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
@@ -2816,7 +2816,8 @@
// 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);
