From 0188c07e524986bd6f8587e57f37f97e3143bb0e Mon Sep 17 00:00:00 2001 From: Pat Gavlin Date: Fri, 16 Sep 2016 18:35:38 -0700 Subject: [PATCH] Mark GT_IND nodes that represent VSD targets. The operand to such a node must be materialized into a register on certain platforms. This change defines a new IND-specific flag, GTF_IND_VSD_TGT, and uses that flag to indicate that a particular GT_IND node represents a VSD target. This flag is then observed by lowering on the necessary platforms in order to short circuit the logic that attempts to make the GT_IND's operand contained. --- src/jit/gentree.h | 4 ++++ src/jit/lower.cpp | 4 ++++ src/jit/lowerxarch.cpp | 59 +++++++++++++++++++++---------------------------- src/jit/rationalize.cpp | 5 +++++ src/jit/target.h | 1 + 5 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/jit/gentree.h b/src/jit/gentree.h index e0e6a1a..ab20f57 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -865,6 +865,10 @@ public: #define GTF_IND_TLS_REF 0x08000000 // GT_IND -- the target is accessed via TLS #define GTF_IND_ASG_LHS 0x04000000 // GT_IND -- this GT_IND node is (the effective val) of the LHS of an // assignment; don't evaluate it independently. +#define GTF_IND_VSD_TGT GTF_IND_ASG_LHS // GT_IND -- this GT_IND node represents the target of an indirect virtual + // stub call. This is only valid in the backend, where + // GTF_IND_ASG_LHS is not necessary (all such indirections will + // be lowered to GT_STOREIND). #define GTF_IND_UNALIGNED 0x02000000 // GT_IND -- the load or store is unaligned (we assume worst case // alignment of 1 byte) #define GTF_IND_INVARIANT 0x01000000 // GT_IND -- the target is invariant (a prejit indirection) diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index f75d0af..be3429a 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -3156,6 +3156,8 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call) // All we have to do here is add an indirection to generate the actual call target. GenTree* ind = Ind(call->gtCallAddr); + ind->gtFlags |= GTF_IND_VSD_TGT; + BlockRange().InsertAfter(call->gtCallAddr, ind); call->gtCallAddr = ind; } @@ -3194,7 +3196,9 @@ GenTree* Lowering::LowerVirtualStubCall(GenTreeCall* call) #ifndef _TARGET_X86_ // on x64 we must materialize the target using specific registers. addr->gtRegNum = REG_VIRTUAL_STUB_PARAM; + indir->gtRegNum = REG_JUMP_THUNK_PARAM; + indir->gtFlags |= GTF_IND_VSD_TGT; #endif result = indir; } diff --git a/src/jit/lowerxarch.cpp b/src/jit/lowerxarch.cpp index b3c19d3..1c1f418 100644 --- a/src/jit/lowerxarch.cpp +++ b/src/jit/lowerxarch.cpp @@ -1128,7 +1128,7 @@ void Lowering::TreeNodeInfoInitCall(GenTreeCall* call) { assert(ctrlExpr->isIndir()); - ctrlExpr->gtGetOp1()->gtLsraInfo.setSrcCandidates(l, REG_VIRTUAL_STUB_TARGET); + ctrlExpr->gtGetOp1()->gtLsraInfo.setSrcCandidates(l, RBM_VIRTUAL_STUB_TARGET); MakeSrcContained(call, ctrlExpr); } else @@ -2752,7 +2752,6 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree) GenTreePtr index = nullptr; unsigned mul, cns; bool rev; - bool modifiedSources = false; #ifdef FEATURE_SIMD // If indirTree is of TYP_SIMD12, don't mark addr as contained @@ -2783,16 +2782,22 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree) } #endif // FEATURE_SIMD - // These nodes go into an addr mode: - // - GT_CLS_VAR_ADDR turns into a constant. - // - GT_LCL_VAR_ADDR is a stack addr mode. - if ((addr->OperGet() == GT_CLS_VAR_ADDR) || (addr->OperGet() == GT_LCL_VAR_ADDR)) + if ((indirTree->gtFlags & GTF_IND_VSD_TGT) != 0) { + // The address of an indirection that is marked as the target of a VSD call must + // be computed into a register. Skip any further processing that might otherwise + // make it contained. + } + else if ((addr->OperGet() == GT_CLS_VAR_ADDR) || (addr->OperGet() == GT_LCL_VAR_ADDR)) + { + // These nodes go into an addr mode: + // - GT_CLS_VAR_ADDR turns into a constant. + // - GT_LCL_VAR_ADDR is a stack addr mode. + // make this contained, it turns into a constant that goes into an addr mode MakeSrcContained(indirTree, addr); } - else if (addr->IsCnsIntOrI() && addr->AsIntConCommon()->FitsInAddrBase(comp) && - addr->gtLsraInfo.getDstCandidates(m_lsra) != RBM_VIRTUAL_STUB_PARAM) + else if (addr->IsCnsIntOrI() && addr->AsIntConCommon()->FitsInAddrBase(comp)) { // Amd64: // We can mark any pc-relative 32-bit addr as containable, except for a direct VSD call address. @@ -2814,17 +2819,10 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree) } else if (addr->OperGet() == GT_LEA) { - GenTreeAddrMode* lea = addr->AsAddrMode(); - base = lea->Base(); - index = lea->Index(); - - m_lsra->clearOperandCounts(addr); - // The srcCount is decremented because addr is now "contained", - // then we account for the base and index below, if they are non-null. - info->srcCount--; + MakeSrcContained(indirTree, addr); } else if (comp->codeGen->genCreateAddrMode(addr, -1, true, 0, &rev, &base, &index, &mul, &cns, true /*nogen*/) && - !(modifiedSources = AreSourcesPossiblyModified(indirTree, base, index))) + !AreSourcesPossiblyModified(indirTree, base, index)) { // An addressing mode will be constructed that may cause some // nodes to not need a register, and cause others' lifetimes to be extended @@ -2891,7 +2889,16 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree) } } assert(foundBase && foundIndex); - info->srcCount--; // it gets incremented below. + + if (base != nullptr) + { + MakeSrcContained(indirTree, base); + } + + if (index != nullptr) + { + MakeSrcContained(indirTree, index); + } } else if (addr->gtOper == GT_ARR_ELEM) { @@ -2904,22 +2911,6 @@ void Lowering::SetIndirAddrOpCounts(GenTreePtr indirTree) assert(addr->gtLsraInfo.srcCount >= 2); addr->gtLsraInfo.srcCount -= 1; } - else - { - // it is nothing but a plain indir - info->srcCount--; // base gets added in below - base = addr; - } - - if (base != nullptr) - { - info->srcCount++; - } - - if (index != nullptr && !modifiedSources) - { - info->srcCount++; - } } void Lowering::TreeNodeInfoInitCmp(GenTreePtr tree) diff --git a/src/jit/rationalize.cpp b/src/jit/rationalize.cpp index 11cba06..19a5984 100644 --- a/src/jit/rationalize.cpp +++ b/src/jit/rationalize.cpp @@ -662,6 +662,11 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, ArrayStackgtFlags &= ~GTF_IND_ASG_LHS; + break; + case GT_NOP: // fgMorph sometimes inserts NOP nodes between defs and uses // supposedly 'to prevent constant folding'. In this case, remove the diff --git a/src/jit/target.h b/src/jit/target.h index 3111483..cbf9d8a 100644 --- a/src/jit/target.h +++ b/src/jit/target.h @@ -600,6 +600,7 @@ typedef unsigned short regPairNoSmall; // arm: need 12 bits // VSD target address register #define REG_VIRTUAL_STUB_TARGET REG_EAX + #define RBM_VIRTUAL_STUB_TARGET RBM_EAX // Registers used by PInvoke frame setup #define REG_PINVOKE_FRAME REG_EDI // EDI is p/invoke "Frame" pointer argument to CORINFO_HELP_INIT_PINVOKE_FRAME helper -- 2.7.4