From 3dd6cf1dffb4a67c797d566c1d04daef41ef35f4 Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Wed, 14 Feb 2018 09:39:21 -0800 Subject: [PATCH] Add VSD additional param for DIRECT calls. (#16267) * add a virtualStubParamInfo as an argument * try to delete duplicates expect many failures for arm in lower because we delete: // Change the call type, so we can add the extra indirection here, rather than in codegen * Revert legacy workaround from lower. * Fix GetNonStandardAddedArgCount --- src/jit/compiler.h | 1 + src/jit/gentree.cpp | 21 +++++------ src/jit/lower.cpp | 52 +++------------------------ src/jit/morph.cpp | 102 +++++++++++++++++++++++++++++----------------------- 4 files changed, 71 insertions(+), 105 deletions(-) diff --git a/src/jit/compiler.h b/src/jit/compiler.h index cb34563..b06aee9 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -4920,6 +4920,7 @@ private: bool fgCanFastTailCall(GenTreeCall* call); bool fgCheckStmtAfterTailCall(); void fgMorphTailCall(GenTreeCall* call); + GenTree* fgGetStubAddrArg(GenTreeCall* call); void fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCall* recursiveTailCall); GenTree* fgAssignRecursiveCallArgToCallerParam(GenTree* arg, fgArgTabEntry* argTabEntry, diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index eb13bf9..284b420 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -1043,19 +1043,16 @@ int GenTreeCall::GetNonStandardAddedArgCount(Compiler* compiler) const // R11 = PInvoke cookie param return 1; } - else if (gtCallType == CT_INDIRECT) + else if (IsVirtualStub()) { - if (IsVirtualStub()) - { - // R11 = Virtual stub param - return 1; - } - else if (gtCallCookie != nullptr) - { - // R10 = PInvoke target param - // R11 = PInvoke cookie param - return 2; - } + // R11 = Virtual stub param + return 1; + } + else if ((gtCallType == CT_INDIRECT) && (gtCallCookie != nullptr)) + { + // R10 = PInvoke target param + // R11 = PInvoke cookie param + return 2; } return 0; } diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index 21da582..28258b1 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -4178,23 +4178,9 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call) // fgMorphArgs will have created trees to pass the address in VirtualStubParam.reg. // All we have to do here is add an indirection to generate the actual call target. - GenTree* ind; - -#ifdef _TARGET_ARM_ - // For ARM, fgMorphTailCall has already made gtCallAddr a GT_IND for virtual stub tail calls. - // (When we eliminate LEGACY_BACKEND maybe we can eliminate this asymmetry?) - if (call->IsTailCallViaHelper()) - { - ind = call->gtCallAddr; - assert(ind->gtOper == GT_IND); - } - else -#endif // _TARGET_ARM_ - { - ind = Ind(call->gtCallAddr); - BlockRange().InsertAfter(call->gtCallAddr, ind); - call->gtCallAddr = ind; - } + GenTree* ind = Ind(call->gtCallAddr); + BlockRange().InsertAfter(call->gtCallAddr, ind); + call->gtCallAddr = ind; ind->gtFlags |= GTF_IND_REQ_ADDR_IN_REG; @@ -4227,37 +4213,7 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call) if (result == nullptr) { - GenTree* indir = Ind(addr); - -// On x86 we generate this: -// call dword ptr [rel32] ; FF 15 ---rel32---- -// So we don't use a register. -#ifndef _TARGET_X86_ - // on x64 we must materialize the target using specific registers. - addr->gtRegNum = comp->virtualStubParamInfo->GetReg(); - -// On ARM we must use a proper address in R12(thunk register) without dereferencing. -// So for the jump we use the default register. -// TODO: specifying register probably unnecessary for other platforms, too. -#if !defined(_TARGET_UNIX_) && !defined(_TARGET_ARM_) && !defined(_TARGET_ARM64_) - indir->gtRegNum = REG_JUMP_THUNK_PARAM; -#elif defined(_TARGET_ARM64_) - // Prevent indir->gtRegNum from colliding with addr->gtRegNum - indir->gtRegNum = REG_JUMP_THUNK_PARAM; - - // Sanity checks - assert(addr->gtRegNum != indir->gtRegNum); // indir and addr registers must be different - static_assert_no_msg((RBM_JUMP_THUNK_PARAM & RBM_ARG_REGS) == 0); - static_assert_no_msg((RBM_JUMP_THUNK_PARAM & RBM_INT_CALLEE_TRASH) != 0); - -#elif defined(_TARGET_ARM_) - // TODO-ARM-Cleanup: This is a temporarey hotfix to fix a regression observed in Linux/ARM. - if (!comp->IsTargetAbi(CORINFO_CORERT_ABI)) - indir->gtRegNum = REG_JUMP_THUNK_PARAM; -#endif - indir->gtFlags |= GTF_IND_REQ_ADDR_IN_REG; -#endif - result = indir; + result = Ind(addr); } } diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index 4f1e65f..3b3247e 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -3044,7 +3044,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) assert(arg2 != nullptr); nonStandardArgs.Add(arg2, REG_LNGARG_HI); } -#else // !defined(_TARGET_X86_) +#else // !_TARGET_X86_ // TODO-X86-CQ: Currently RyuJIT/x86 passes args on the stack, so this is not needed. // If/when we change that, the following code needs to be changed to correctly support the (TBD) managed calling // convention for x86/SSE. @@ -3079,38 +3079,27 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) nonStandardArgs.Add(cns, REG_PINVOKE_COOKIE_PARAM); } - else if (call->IsVirtualStub() && (call->gtCallType == CT_INDIRECT) && !call->IsTailCallViaHelper()) + else if (call->IsVirtualStub()) { - // indirect VSD stubs need the base of the indirection cell to be - // passed in addition. At this point that is the value in gtCallAddr. - // The actual call target will be derived from gtCallAddr in call - // lowering. - - // If it is a VSD call getting dispatched via tail call helper, - // fgMorphTailCall() would materialize stub addr as an additional - // parameter added to the original arg list and hence no need to - // add as a non-standard arg. - - GenTree* arg = call->gtCallAddr; - if (arg->OperIsLocal()) + if (!call->IsTailCallViaHelper()) { - arg = gtClone(arg, true); + GenTree* stubAddrArg = fgGetStubAddrArg(call); + // And push the stub address onto the list of arguments + call->gtCallArgs = gtNewListNode(stubAddrArg, call->gtCallArgs); + + numArgs++; + nonStandardArgs.Add(stubAddrArg, stubAddrArg->gtRegNum); } else { - call->gtCallAddr = fgInsertCommaFormTemp(&arg); - call->gtFlags |= GTF_ASG; + // If it is a VSD call getting dispatched via tail call helper, + // fgMorphTailCall() would materialize stub addr as an additional + // parameter added to the original arg list and hence no need to + // add as a non-standard arg. } - noway_assert(arg != nullptr); - - // And push the stub address onto the list of arguments - call->gtCallArgs = gtNewListNode(arg, call->gtCallArgs); - numArgs++; - - nonStandardArgs.Add(arg, virtualStubParamInfo->GetReg()); } else -#endif // defined(_TARGET_X86_) +#endif // !_TARGET_X86_ if (call->gtCallType == CT_INDIRECT && (call->gtCallCookie != nullptr)) { assert(!call->IsUnmanaged()); @@ -7749,12 +7738,12 @@ void Compiler::fgMorphTailCall(GenTreeCall* call) if (call->IsVirtualStub()) { flags = CORINFO_TAILCALL_STUB_DISPATCH_ARG; - +#ifdef LEGACY_BACKEND GenTree* arg; if (call->gtCallType == CT_INDIRECT) { arg = gtClone(call->gtCallAddr, true); - noway_assert(arg != NULL); + noway_assert(arg != nullptr); } else { @@ -7767,12 +7756,17 @@ void Compiler::fgMorphTailCall(GenTreeCall* call) call->gtStubCallStubAddr = NULL; call->gtCallType = CT_INDIRECT; } + arg->gtRegNum = virtualStubParamInfo->GetReg(); // Add the extra indirection to generate the real target call->gtCallAddr = gtNewOperNode(GT_IND, TYP_I_IMPL, call->gtCallAddr); call->gtFlags |= GTF_EXCEPT; + call->gtCallArgs = gtNewListNode(arg, call->gtCallArgs); +#else // !LEGACY_BACKEND + GenTree* stubAddrArg = fgGetStubAddrArg(call); // And push the stub address onto the list of arguments - call->gtCallArgs = gtNewListNode(arg, call->gtCallArgs); + call->gtCallArgs = gtNewListNode(stubAddrArg, call->gtCallArgs); +#endif // !LEGACY_BACKEND } else if (call->IsVirtualVtable()) { @@ -7832,7 +7826,7 @@ void Compiler::fgMorphTailCall(GenTreeCall* call) GenTree* arg = new (this, GT_NOP) GenTreeOp(GT_NOP, TYP_I_IMPL); codeGen->genMarkTreeInReg(arg, REG_TAILCALL_ADDR); #else // !LEGACY_BACKEND - GenTree* arg = gtNewIconNode(0, TYP_I_IMPL); + GenTree* arg = gtNewIconNode(0, TYP_I_IMPL); #endif // !LEGACY_BACKEND call->gtCallArgs = gtNewListNode(arg, call->gtCallArgs); @@ -8012,24 +8006,10 @@ void Compiler::fgMorphTailCall(GenTreeCall* call) CorInfoHelperTailCallSpecialHandling flags = CorInfoHelperTailCallSpecialHandling(0); if (call->IsVirtualStub()) { - GenTree* stubAddrArg; - flags = CORINFO_TAILCALL_STUB_DISPATCH_ARG; - if (call->gtCallType == CT_INDIRECT) - { - stubAddrArg = gtClone(call->gtCallAddr, true); - noway_assert(stubAddrArg != nullptr); - } - else - { - noway_assert((call->gtCallMoreFlags & GTF_CALL_M_VIRTSTUB_REL_INDIRECT) != 0); - - ssize_t addr = ssize_t(call->gtStubCallStubAddr); - stubAddrArg = gtNewIconHandleNode(addr, GTF_ICON_FTN_ADDR); - } - - // Push the stub address onto the list of arguments + GenTree* stubAddrArg = fgGetStubAddrArg(call); + // And push the stub address onto the list of arguments call->gtCallArgs = gtNewListNode(stubAddrArg, call->gtCallArgs); } @@ -8090,6 +8070,38 @@ void Compiler::fgMorphTailCall(GenTreeCall* call) DISPTREE(call); } +//------------------------------------------------------------------------ +// fgGetStubAddrArg: Return the virtual stub address for the given call. +// +// Notes: +// the JIT must place the address of the stub used to load the call target, +// the "stub indirection cell", in special call argument with special register. +// +// Arguments: +// call - a call that needs virtual stub dispatching. +// +// Return Value: +// addr tree with set resister requirements. +// +GenTree* Compiler::fgGetStubAddrArg(GenTreeCall* call) +{ + assert(call->IsVirtualStub()); + GenTree* stubAddrArg; + if (call->gtCallType == CT_INDIRECT) + { + stubAddrArg = gtClone(call->gtCallAddr, true); + } + else + { + assert(call->gtCallMoreFlags & GTF_CALL_M_VIRTSTUB_REL_INDIRECT); + ssize_t addr = ssize_t(call->gtStubCallStubAddr); + stubAddrArg = gtNewIconHandleNode(addr, GTF_ICON_FTN_ADDR); + } + assert(stubAddrArg != nullptr); + stubAddrArg->gtRegNum = virtualStubParamInfo->GetReg(); + return stubAddrArg; +} + //------------------------------------------------------------------------------ // fgMorphRecursiveFastTailCallIntoLoop : Transform a recursive fast tail call into a loop. // -- 2.7.4