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 <[email protected]> Kokoro-Presubmit: kokoro <[email protected]> Reviewed-by: Nicolas Capens <[email protected]>
raochp · Feb 27, 2020 · ad3e42a · ad3e42a
1 parent 4b34ee3
commit ad3e42a
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 24 deletions.
diff --git 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 @@ class ELFMemoryStreamer : public Ice::ELFStreamer, public Routine
 #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
@@ -426,40 +426,43 @@ 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.
     constexpr bool HasSideEffects = true;
     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); }
   Operand *getArg(SizeT I) const { return getSrc(I + 1); }
   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; }
   Type getReturnType() const;
 
 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);
   }
 
 private:
   const bool HasTailCall;
   const bool IsTargetHelperCall;
+  const bool IsVariadic;
 };
 
 /// Cast instruction (a.k.a. conversion operation).
@@ -633,7 +636,7 @@ class InstIntrinsicCall : public InstCall {
 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
@@ -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
@@ -62,7 +62,8 @@ class TargetX8632 final : public ::Ice::X8632::TargetX86Base<X8632::Traits> {
   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
@@ -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
@@ -65,7 +65,8 @@ class TargetX8664 final : public X8664::TargetX86Base<X8664::Traits> {
   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
@@ -384,7 +384,8 @@ template <typename TraitsType> class TargetX86Base : public TargetLowering {
 
   /// 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
@@ -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);