From fdb900e605a4820a93d28709b97d6bab852726ef Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 8 Jul 2022 10:42:08 +0200 Subject: [PATCH] JIT: Remove some fgWalkTree uses (#71767) Convert them to GenTreeVisitor. --- src/coreclr/jit/compiler.h | 34 +- src/coreclr/jit/fginline.cpp | 1105 +++++++++++++++++---------------- src/coreclr/jit/gentree.cpp | 202 +++--- src/coreclr/jit/optcse.cpp | 53 +- src/coreclr/jit/optimizer.cpp | 235 +++---- 5 files changed, 837 insertions(+), 792 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8503b0250157b..8aa80281b663e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2702,8 +2702,6 @@ class Compiler void gtUpdateNodeOperSideEffects(GenTree* tree); - void gtUpdateNodeOperSideEffectsPost(GenTree* tree); - // Returns "true" iff the complexity (not formally defined, but first interpretation // is #of nodes in subtree) of "tree" is greater than "limit". // (This is somewhat redundant with the "GetCostEx()/GetCostSz()" fields, but can be used @@ -5930,9 +5928,6 @@ class Compiler bool fgForwardSubBlock(BasicBlock* block); bool fgForwardSubStatement(Statement* statement); - static fgWalkPreFn fgUpdateSideEffectsPre; - static fgWalkPostFn fgUpdateSideEffectsPost; - // The given local variable, required to be a struct variable, is being assigned via // a "lclField", to make it masquerade as an integral type in the ABI. Make sure that // the variable is not enregistered, and is therefore not promoted independently. @@ -6468,7 +6463,7 @@ class Compiler int arrayLengthCount; }; - static fgWalkResult optInvertCountTreeInfo(GenTree** pTree, fgWalkData* data); + OptInvertCountTreeInfoType optInvertCountTreeInfo(GenTree* tree); bool optInvertWhileLoop(BasicBlock* block); @@ -6486,9 +6481,19 @@ class Compiler bool dupCond, unsigned* iterCount); - static fgWalkPreFn optIsVarAssgCB; - protected: + struct isVarAssgDsc + { + GenTree* ivaSkip; + ALLVARSET_TP ivaMaskVal; // Set of variables assigned to. This is a set of all vars, not tracked vars. + unsigned ivaVar; // Variable we are interested in, or -1 + varRefKinds ivaMaskInd; // What kind of indirect assignments are there? + callInterf ivaMaskCall; // What kind of calls are there? + bool ivaMaskIncomplete; // Variables not representable in ivaMaskVal were assigned to. + }; + + bool optIsVarAssignedWithDesc(Statement* stmt, isVarAssgDsc* dsc); + bool optIsVarAssigned(BasicBlock* beg, BasicBlock* end, GenTree* skip, unsigned var); bool optIsVarAssgLoop(unsigned lnum, unsigned var); @@ -6735,19 +6740,6 @@ class Compiler void optOptimizeCSEs(); - struct isVarAssgDsc - { - GenTree* ivaSkip; - ALLVARSET_TP ivaMaskVal; // Set of variables assigned to. This is a set of all vars, not tracked vars. -#ifdef DEBUG - void* ivaSelf; -#endif - unsigned ivaVar; // Variable we are interested in, or -1 - varRefKinds ivaMaskInd; // What kind of indirect assignments are there? - callInterf ivaMaskCall; // What kind of calls are there? - bool ivaMaskIncomplete; // Variables not representable in ivaMaskVal were assigned to. - }; - static callInterf optCallInterf(GenTreeCall* call); public: diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 642a7a83a4e9b..c28c48e0c67b6 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -62,6 +62,566 @@ unsigned Compiler::fgCheckInlineDepthAndRecursion(InlineInfo* inlineInfo) return depth; } +class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor +{ + bool m_madeChanges = false; + +public: + enum + { + DoPreOrder = true, + DoPostOrder = true, + UseExecutionOrder = true, + }; + + SubstitutePlaceholdersAndDevirtualizeWalker(Compiler* comp) : GenTreeVisitor(comp) + { + } + + bool MadeChanges() + { + return m_madeChanges; + } + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* tree = *use; + + // All the operations here and in the corresponding postorder + // callback (LateDevirtualization) are triggered by GT_CALL or + // GT_RET_EXPR trees, and these (should) have the call side + // effect flag. + // + // So bail out for any trees that don't have this flag. + if ((tree->gtFlags & GTF_CALL) == 0) + { + return fgWalkResult::WALK_SKIP_SUBTREES; + } + + if (tree->OperIs(GT_RET_EXPR)) + { + UpdateInlineReturnExpressionPlaceHolder(tree, user); + } + +#if FEATURE_MULTIREG_RET +#if defined(DEBUG) + + // Make sure we don't have a tree like so: V05 = (, , , retExpr); + // Since we only look one level above for the parent for '=' and + // do not check if there is a series of COMMAs. See above. + // Importer and FlowGraph will not generate such a tree, so just + // leaving an assert in here. This can be fixed by looking ahead + // when we visit GT_ASG similar to AttachStructInlineeToAsg. + // + if (tree->OperGet() == GT_ASG) + { + GenTree* value = tree->AsOp()->gtOp2; + + if (value->OperGet() == GT_COMMA) + { + GenTree* effectiveValue = value->gtEffectiveVal(/*commaOnly*/ true); + + noway_assert(!varTypeIsStruct(effectiveValue) || (effectiveValue->OperGet() != GT_RET_EXPR) || + !m_compiler->IsMultiRegReturnedType(effectiveValue->AsRetExpr()->gtRetClsHnd, + CorInfoCallConvExtension::Managed)); + } + } + +#endif // defined(DEBUG) +#endif // FEATURE_MULTIREG_RET + return fgWalkResult::WALK_CONTINUE; + } + + fgWalkResult PostOrderVisit(GenTree** use, GenTree* user) + { + LateDevirtualization(use, user); + return fgWalkResult::WALK_CONTINUE; + } + +private: + //------------------------------------------------------------------------ + // UpdateInlineReturnExpressionPlaceHolder: replace an + // inline return expression placeholder if there is one. + // + // Arguments: + // pTree -- pointer to tree to examine for updates + // data -- context data for the tree walk + // + // Returns: + // fgWalkResult indicating the walk should continue; that + // is we wish to fully explore the tree. + // + // Notes: + // Looks for GT_RET_EXPR nodes that arose from tree splitting done + // during importation for inline candidates, and replaces them. + // + // For successful inlines, substitutes the return value expression + // from the inline body for the GT_RET_EXPR. + // + // For failed inlines, rejoins the original call into the tree from + // whence it was split during importation. + // + // The code doesn't actually know if the corresponding inline + // succeeded or not; it relies on the fact that gtInlineCandidate + // initially points back at the call and is modified in place to + // the inlinee return expression if the inline is successful (see + // tail end of fgInsertInlineeBlocks for the update of iciCall). + // + // If the return type is a struct type and we're on a platform + // where structs can be returned in multiple registers, ensure the + // call has a suitable parent. + // + // If the original call type and the substitution type are different + // the functions makes necessary updates. It could happen if there was + // an implicit conversion in the inlinee body. + // + void UpdateInlineReturnExpressionPlaceHolder(GenTree* tree, GenTree* parent) + { + CORINFO_CLASS_HANDLE retClsHnd = NO_CLASS_HANDLE; + + while (tree->OperIs(GT_RET_EXPR)) + { + // We are going to copy the tree from the inlinee, + // so record the handle now. + // + if (varTypeIsStruct(tree)) + { + retClsHnd = tree->AsRetExpr()->gtRetClsHnd; + } + + // Skip through chains of GT_RET_EXPRs (say from nested inlines) + // to the actual tree to use. + // + BasicBlockFlags bbFlags; + GenTree* inlineCandidate = tree; + do + { + GenTreeRetExpr* retExpr = inlineCandidate->AsRetExpr(); + inlineCandidate = retExpr->gtInlineCandidate; + bbFlags = retExpr->bbFlags; + } while (inlineCandidate->OperIs(GT_RET_EXPR)); + + // We might as well try and fold the return value. Eg returns of + // constant bools will have CASTS. This folding may uncover more + // GT_RET_EXPRs, so we loop around until we've got something distinct. + // + inlineCandidate = m_compiler->gtFoldExpr(inlineCandidate); + var_types retType = tree->TypeGet(); + +#ifdef DEBUG + if (m_compiler->verbose) + { + printf("\nReplacing the return expression placeholder "); + Compiler::printTreeID(tree); + printf(" with "); + Compiler::printTreeID(inlineCandidate); + printf("\n"); + // Dump out the old return expression placeholder it will be overwritten by the ReplaceWith below + m_compiler->gtDispTree(tree); + } +#endif // DEBUG + + var_types newType = inlineCandidate->TypeGet(); + + // If we end up swapping type we may need to retype the tree: + if (retType != newType) + { + if ((retType == TYP_BYREF) && (tree->OperGet() == GT_IND)) + { + // - in an RVA static if we've reinterpreted it as a byref; + assert(newType == TYP_I_IMPL); + JITDUMP("Updating type of the return GT_IND expression to TYP_BYREF\n"); + inlineCandidate->gtType = TYP_BYREF; + } + } + + tree->ReplaceWith(inlineCandidate, m_compiler); + m_madeChanges = true; + m_compiler->compCurBB->bbFlags |= (bbFlags & BBF_SPLIT_GAINED); + +#ifdef DEBUG + if (m_compiler->verbose) + { + printf("\nInserting the inline return expression\n"); + m_compiler->gtDispTree(tree); + printf("\n"); + } +#endif // DEBUG + } + + // If an inline was rejected and the call returns a struct, we may + // have deferred some work when importing call for cases where the + // struct is returned in register(s). + // + // See the bail-out clauses in impFixupCallStructReturn for inline + // candidates. + // + // Do the deferred work now. + if (retClsHnd != NO_CLASS_HANDLE) + { + Compiler::structPassingKind howToReturnStruct; + var_types returnType = + m_compiler->getReturnTypeForStruct(retClsHnd, CorInfoCallConvExtension::Managed, &howToReturnStruct); + + switch (howToReturnStruct) + { + +#if FEATURE_MULTIREG_RET + + // Is this a type that is returned in multiple registers + // or a via a primitve type that is larger than the struct type? + // if so we need to force into into a form we accept. + // i.e. LclVar = call() + case Compiler::SPK_ByValue: + case Compiler::SPK_ByValueAsHfa: + { + // See assert below, we only look one level above for an asg parent. + if (parent->gtOper == GT_ASG) + { + // Either lhs is a call V05 = call(); or lhs is addr, and asg becomes a copyBlk. + AttachStructInlineeToAsg(parent, tree, retClsHnd); + } + else + { + // Just assign the inlinee to a variable to keep it simple. + tree->ReplaceWith(AssignStructInlineeToVar(tree, retClsHnd), m_compiler); + } + m_madeChanges = true; + } + break; + +#endif // FEATURE_MULTIREG_RET + + case Compiler::SPK_EnclosingType: + case Compiler::SPK_PrimitiveType: + // No work needs to be done, the call has struct type and should keep it. + break; + + case Compiler::SPK_ByReference: + // We should have already added the return buffer + // when we first imported the call + break; + + default: + noway_assert(!"Unexpected struct passing kind"); + break; + } + } + } + +#if FEATURE_MULTIREG_RET + + /*************************************************************************************************** + * child - The inlinee of the retExpr node. + * retClsHnd - The struct class handle of the type of the inlinee. + * + * Assign the inlinee to a tmp, if it is a call, just assign it to a lclVar, else we can + * use a copyblock to do the assignment. + */ + GenTree* AssignStructInlineeToVar(GenTree* child, CORINFO_CLASS_HANDLE retClsHnd) + { + assert(child->gtOper != GT_RET_EXPR && child->gtOper != GT_MKREFANY); + + unsigned tmpNum = m_compiler->lvaGrabTemp(false DEBUGARG("RetBuf for struct inline return candidates.")); + m_compiler->lvaSetStruct(tmpNum, retClsHnd, false); + var_types structType = m_compiler->lvaGetDesc(tmpNum)->lvType; + + GenTree* dst = m_compiler->gtNewLclvNode(tmpNum, structType); + + // If we have a call, we'd like it to be: V00 = call(), but first check if + // we have a ", , , call()" -- this is very defensive as we may never get + // an inlinee that is made of commas. If the inlinee is not a call, then + // we use a copy block to do the assignment. + GenTree* src = child; + GenTree* lastComma = nullptr; + while (src->gtOper == GT_COMMA) + { + lastComma = src; + src = src->AsOp()->gtOp2; + } + + GenTree* newInlinee = nullptr; + if (src->gtOper == GT_CALL) + { + // If inlinee was just a call, new inlinee is v05 = call() + newInlinee = m_compiler->gtNewAssignNode(dst, src); + + // When returning a multi-register value in a local var, make sure the variable is + // marked as lvIsMultiRegRet, so it does not get promoted. + if (src->AsCall()->HasMultiRegRetVal()) + { + m_compiler->lvaGetDesc(tmpNum)->lvIsMultiRegRet = true; + } + + // If inlinee was comma, but a deeper call, new inlinee is (, , , v05 = call()) + if (child->gtOper == GT_COMMA) + { + lastComma->AsOp()->gtOp2 = newInlinee; + newInlinee = child; + } + } + else + { + // Inlinee is not a call, so just create a copy block to the tmp. + src = child; + GenTree* dstAddr = GetStructAsStructPtr(dst); + GenTree* srcAddr = GetStructAsStructPtr(src); + newInlinee = m_compiler->gtNewCpObjNode(dstAddr, srcAddr, retClsHnd, false); + } + + GenTree* production = m_compiler->gtNewLclvNode(tmpNum, structType); + return m_compiler->gtNewOperNode(GT_COMMA, structType, newInlinee, production); + } + + /*************************************************************************************************** + * tree - The tree pointer that has one of its child nodes as retExpr. + * child - The inlinee child. + * retClsHnd - The struct class handle of the type of the inlinee. + * + * V04 = call() assignments are okay as we codegen it. Everything else needs to be a copy block or + * would need a temp. For example, a cast(ldobj) will then be, cast(v05 = ldobj, v05); But it is + * a very rare (or impossible) scenario that we'd have a retExpr transform into a ldobj other than + * a lclVar/call. So it is not worthwhile to do pattern matching optimizations like addr(ldobj(op1)) + * can just be op1. + */ + void AttachStructInlineeToAsg(GenTree* tree, GenTree* child, CORINFO_CLASS_HANDLE retClsHnd) + { + // We are okay to have: + // 1. V02 = call(); + // 2. copyBlk(dstAddr, srcAddr); + assert(tree->gtOper == GT_ASG); + + // We have an assignment, we codegen only V05 = call(). + if (child->gtOper == GT_CALL && tree->AsOp()->gtOp1->gtOper == GT_LCL_VAR) + { + // If it is a multireg return on x64/ux, the local variable should be marked as lvIsMultiRegRet + if (child->AsCall()->HasMultiRegRetVal()) + { + unsigned lclNum = tree->AsOp()->gtOp1->AsLclVarCommon()->GetLclNum(); + m_compiler->lvaGetDesc(lclNum)->lvIsMultiRegRet = true; + } + return; + } + + GenTree* dstAddr = GetStructAsStructPtr(tree->AsOp()->gtOp1); + GenTree* srcAddr = GetStructAsStructPtr( + (child->gtOper == GT_CALL) + ? AssignStructInlineeToVar(child, retClsHnd) // Assign to a variable if it is a call. + : child); // Just get the address, if not a call. + + tree->ReplaceWith(m_compiler->gtNewCpObjNode(dstAddr, srcAddr, retClsHnd, false), m_compiler); + } + + /********************************************************************************* + * + * tree - The node which needs to be converted to a struct pointer. + * + * Return the pointer by either __replacing__ the tree node with a suitable pointer + * type or __without replacing__ and just returning a subtree or by __modifying__ + * a subtree. + */ + GenTree* GetStructAsStructPtr(GenTree* tree) + { + noway_assert(tree->OperIs(GT_LCL_VAR, GT_FIELD, GT_IND, GT_BLK, GT_OBJ, GT_COMMA) || + tree->OperIsSimdOrHWintrinsic() || tree->IsCnsVec()); + // GT_CALL, cannot get address of call. + // GT_MKREFANY, inlining should've been aborted due to mkrefany opcode. + // GT_RET_EXPR, cannot happen after fgUpdateInlineReturnExpressionPlaceHolder + + switch (tree->OperGet()) + { + case GT_BLK: + case GT_OBJ: + case GT_IND: + return tree->AsOp()->gtOp1; + + case GT_COMMA: + tree->AsOp()->gtOp2 = GetStructAsStructPtr(tree->AsOp()->gtOp2); + tree->gtType = TYP_BYREF; + return tree; + + default: + return m_compiler->gtNewOperNode(GT_ADDR, TYP_BYREF, tree); + } + } + +#endif // FEATURE_MULTIREG_RET + + //------------------------------------------------------------------------ + // LateDevirtualization: re-examine calls after inlining to see if we + // can do more devirtualization + // + // Arguments: + // pTree -- pointer to tree to examine for updates + // parent -- parent node containing the pTree edge + // + // Returns: + // fgWalkResult indicating the walk should continue; that + // is we wish to fully explore the tree. + // + // Notes: + // We used to check this opportunistically in the preorder callback for + // calls where the `obj` was fed by a return, but we now re-examine + // all calls. + // + // Late devirtualization (and eventually, perhaps, other type-driven + // opts like cast optimization) can happen now because inlining or other + // optimizations may have provided more accurate types than we saw when + // first importing the trees. + // + // It would be nice to screen candidate sites based on the likelihood + // that something has changed. Otherwise we'll waste some time retrying + // an optimization that will just fail again. + void LateDevirtualization(GenTree** pTree, GenTree* parent) + { + GenTree* tree = *pTree; + // In some (rare) cases the parent node of tree will be smashed to a NOP during + // the preorder by AttachStructToInlineeArg. + // + // jit\Methodical\VT\callconv\_il_reljumper3 for x64 linux + // + // If so, just bail out here. + if (tree == nullptr) + { + assert((parent != nullptr) && parent->OperGet() == GT_NOP); + return; + } + + if (tree->OperGet() == GT_CALL) + { + GenTreeCall* call = tree->AsCall(); + bool tryLateDevirt = call->IsVirtual() && (call->gtCallType == CT_USER_FUNC); + +#ifdef DEBUG + tryLateDevirt = tryLateDevirt && (JitConfig.JitEnableLateDevirtualization() == 1); +#endif // DEBUG + + if (tryLateDevirt) + { +#ifdef DEBUG + if (m_compiler->verbose) + { + printf("**** Late devirt opportunity\n"); + m_compiler->gtDispTree(call); + } +#endif // DEBUG + + CORINFO_CONTEXT_HANDLE context = nullptr; + CORINFO_METHOD_HANDLE method = call->gtCallMethHnd; + unsigned methodFlags = 0; + const bool isLateDevirtualization = true; + const bool explicitTailCall = call->IsTailPrefixedCall(); + + if ((call->gtCallMoreFlags & GTF_CALL_M_HAS_LATE_DEVIRT_INFO) != 0) + { + context = call->gtLateDevirtualizationInfo->exactContextHnd; + // Note: we might call this multiple times for the same trees. + // If the devirtualization below succeeds, the call becomes + // non-virtual and we won't get here again. If it does not + // succeed we might get here again so we keep the late devirt + // info. + } + + m_compiler->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &context, nullptr, + isLateDevirtualization, explicitTailCall); + m_madeChanges = true; + } + } + else if (tree->OperGet() == GT_ASG) + { + // If we're assigning to a ref typed local that has one definition, + // we may be able to sharpen the type for the local. + GenTree* const effLhs = tree->gtGetOp1()->gtEffectiveVal(); + + if ((effLhs->OperGet() == GT_LCL_VAR) && (effLhs->TypeGet() == TYP_REF)) + { + const unsigned lclNum = effLhs->AsLclVarCommon()->GetLclNum(); + LclVarDsc* lcl = m_compiler->lvaGetDesc(lclNum); + + if (lcl->lvSingleDef) + { + GenTree* rhs = tree->gtGetOp2(); + bool isExact = false; + bool isNonNull = false; + CORINFO_CLASS_HANDLE newClass = m_compiler->gtGetClassHandle(rhs, &isExact, &isNonNull); + + if (newClass != NO_CLASS_HANDLE) + { + m_compiler->lvaUpdateClass(lclNum, newClass, isExact); + m_madeChanges = true; + } + } + } + + // If we created a self-assignment (say because we are sharing return spill temps) + // we can remove it. + // + GenTree* const lhs = tree->gtGetOp1(); + GenTree* const rhs = tree->gtGetOp2(); + if (lhs->OperIs(GT_LCL_VAR) && GenTree::Compare(lhs, rhs)) + { + m_compiler->gtUpdateNodeSideEffects(tree); + assert((tree->gtFlags & GTF_SIDE_EFFECT) == GTF_ASG); + JITDUMP("... removing self-assignment\n"); + DISPTREE(tree); + tree->gtBashToNOP(); + m_madeChanges = true; + } + } + else if (tree->OperGet() == GT_JTRUE) + { + // See if this jtrue is now foldable. + BasicBlock* block = m_compiler->compCurBB; + GenTree* condTree = tree->AsOp()->gtOp1; + assert(tree == block->lastStmt()->GetRootNode()); + + if (condTree->OperGet() == GT_CNS_INT) + { + JITDUMP(" ... found foldable jtrue at [%06u] in " FMT_BB "\n", m_compiler->dspTreeID(tree), + block->bbNum); + noway_assert((block->bbNext->countOfInEdges() > 0) && (block->bbJumpDest->countOfInEdges() > 0)); + + // We have a constant operand, and should have the all clear to optimize. + // Update side effects on the tree, assert there aren't any, and bash to nop. + m_compiler->gtUpdateNodeSideEffects(tree); + assert((tree->gtFlags & GTF_SIDE_EFFECT) == 0); + tree->gtBashToNOP(); + m_madeChanges = true; + + BasicBlock* bNotTaken = nullptr; + + if (condTree->AsIntCon()->gtIconVal != 0) + { + block->bbJumpKind = BBJ_ALWAYS; + bNotTaken = block->bbNext; + } + else + { + block->bbJumpKind = BBJ_NONE; + bNotTaken = block->bbJumpDest; + } + + m_compiler->fgRemoveRefPred(bNotTaken, block); + + // If that was the last ref, a subsequent flow-opt pass + // will clean up the now-unreachable bNotTaken, and any + // other transitively unreachable blocks. + if (bNotTaken->bbRefs == 0) + { + JITDUMP("... it looks like " FMT_BB " is now unreachable!\n", bNotTaken->bbNum); + } + } + } + else + { + const var_types retType = tree->TypeGet(); + GenTree* foldedTree = m_compiler->gtFoldExpr(tree); + *pTree = foldedTree; + m_madeChanges = true; + } + } +}; + //------------------------------------------------------------------------ // fgInline - expand inline candidates // @@ -104,8 +664,9 @@ PhaseStatus Compiler::fgInline() noway_assert(fgFirstBB != nullptr); - BasicBlock* block = fgFirstBB; - bool madeChanges = false; + BasicBlock* block = fgFirstBB; + SubstitutePlaceholdersAndDevirtualizeWalker walker(this); + bool madeChanges = false; do { @@ -124,7 +685,7 @@ PhaseStatus Compiler::fgInline() // See if we need to replace some return value place holders. // Also, see if this replacement enables further devirtualization. // - // Note we have both preorder and postorder callbacks here. + // Note we are doing both preorder and postorder work in this walker. // // The preorder callback is responsible for replacing GT_RET_EXPRs // with the appropriate expansion (call or inline result). @@ -135,8 +696,7 @@ PhaseStatus Compiler::fgInline() // possible further optimization, as the (now complete) GT_RET_EXPR // replacement may have enabled optimizations by providing more // specific types for trees or variables. - fgWalkTree(stmt->GetRootNodePointer(), fgUpdateInlineReturnExpressionPlaceHolder, fgLateDevirtualization, - &madeChanges); + walker.WalkTree(stmt->GetRootNodePointer(), nullptr); GenTree* expr = stmt->GetRootNode(); @@ -192,6 +752,8 @@ PhaseStatus Compiler::fgInline() } while (block); + madeChanges |= walker.MadeChanges(); + #ifdef DEBUG // Check that we should not have any inline candidate or return value place holder left. @@ -297,539 +859,6 @@ void Compiler::fgNoteNonInlineCandidate(Statement* stmt, GenTreeCall* call) #endif -#if FEATURE_MULTIREG_RET - -/********************************************************************************* - * - * tree - The node which needs to be converted to a struct pointer. - * - * Return the pointer by either __replacing__ the tree node with a suitable pointer - * type or __without replacing__ and just returning a subtree or by __modifying__ - * a subtree. - */ -GenTree* Compiler::fgGetStructAsStructPtr(GenTree* tree) -{ - noway_assert(tree->OperIs(GT_LCL_VAR, GT_FIELD, GT_IND, GT_BLK, GT_OBJ, GT_COMMA) || - tree->OperIsSimdOrHWintrinsic() || tree->IsCnsVec()); - // GT_CALL, cannot get address of call. - // GT_MKREFANY, inlining should've been aborted due to mkrefany opcode. - // GT_RET_EXPR, cannot happen after fgUpdateInlineReturnExpressionPlaceHolder - - switch (tree->OperGet()) - { - case GT_BLK: - case GT_OBJ: - case GT_IND: - return tree->AsOp()->gtOp1; - - case GT_COMMA: - tree->AsOp()->gtOp2 = fgGetStructAsStructPtr(tree->AsOp()->gtOp2); - tree->gtType = TYP_BYREF; - return tree; - - default: - return gtNewOperNode(GT_ADDR, TYP_BYREF, tree); - } -} - -/*************************************************************************************************** - * child - The inlinee of the retExpr node. - * retClsHnd - The struct class handle of the type of the inlinee. - * - * Assign the inlinee to a tmp, if it is a call, just assign it to a lclVar, else we can - * use a copyblock to do the assignment. - */ -GenTree* Compiler::fgAssignStructInlineeToVar(GenTree* child, CORINFO_CLASS_HANDLE retClsHnd) -{ - assert(child->gtOper != GT_RET_EXPR && child->gtOper != GT_MKREFANY); - - unsigned tmpNum = lvaGrabTemp(false DEBUGARG("RetBuf for struct inline return candidates.")); - lvaSetStruct(tmpNum, retClsHnd, false); - var_types structType = lvaTable[tmpNum].lvType; - - GenTree* dst = gtNewLclvNode(tmpNum, structType); - - // If we have a call, we'd like it to be: V00 = call(), but first check if - // we have a ", , , call()" -- this is very defensive as we may never get - // an inlinee that is made of commas. If the inlinee is not a call, then - // we use a copy block to do the assignment. - GenTree* src = child; - GenTree* lastComma = nullptr; - while (src->gtOper == GT_COMMA) - { - lastComma = src; - src = src->AsOp()->gtOp2; - } - - GenTree* newInlinee = nullptr; - if (src->gtOper == GT_CALL) - { - // If inlinee was just a call, new inlinee is v05 = call() - newInlinee = gtNewAssignNode(dst, src); - - // When returning a multi-register value in a local var, make sure the variable is - // marked as lvIsMultiRegRet, so it does not get promoted. - if (src->AsCall()->HasMultiRegRetVal()) - { - lvaTable[tmpNum].lvIsMultiRegRet = true; - } - - // If inlinee was comma, but a deeper call, new inlinee is (, , , v05 = call()) - if (child->gtOper == GT_COMMA) - { - lastComma->AsOp()->gtOp2 = newInlinee; - newInlinee = child; - } - } - else - { - // Inlinee is not a call, so just create a copy block to the tmp. - src = child; - GenTree* dstAddr = fgGetStructAsStructPtr(dst); - GenTree* srcAddr = fgGetStructAsStructPtr(src); - newInlinee = gtNewCpObjNode(dstAddr, srcAddr, retClsHnd, false); - } - - GenTree* production = gtNewLclvNode(tmpNum, structType); - return gtNewOperNode(GT_COMMA, structType, newInlinee, production); -} - -/*************************************************************************************************** - * tree - The tree pointer that has one of its child nodes as retExpr. - * child - The inlinee child. - * retClsHnd - The struct class handle of the type of the inlinee. - * - * V04 = call() assignments are okay as we codegen it. Everything else needs to be a copy block or - * would need a temp. For example, a cast(ldobj) will then be, cast(v05 = ldobj, v05); But it is - * a very rare (or impossible) scenario that we'd have a retExpr transform into a ldobj other than - * a lclVar/call. So it is not worthwhile to do pattern matching optimizations like addr(ldobj(op1)) - * can just be op1. - */ -void Compiler::fgAttachStructInlineeToAsg(GenTree* tree, GenTree* child, CORINFO_CLASS_HANDLE retClsHnd) -{ - // We are okay to have: - // 1. V02 = call(); - // 2. copyBlk(dstAddr, srcAddr); - assert(tree->gtOper == GT_ASG); - - // We have an assignment, we codegen only V05 = call(). - if (child->gtOper == GT_CALL && tree->AsOp()->gtOp1->gtOper == GT_LCL_VAR) - { - // If it is a multireg return on x64/ux, the local variable should be marked as lvIsMultiRegRet - if (child->AsCall()->HasMultiRegRetVal()) - { - unsigned lclNum = tree->AsOp()->gtOp1->AsLclVarCommon()->GetLclNum(); - lvaTable[lclNum].lvIsMultiRegRet = true; - } - return; - } - - GenTree* dstAddr = fgGetStructAsStructPtr(tree->AsOp()->gtOp1); - GenTree* srcAddr = fgGetStructAsStructPtr( - (child->gtOper == GT_CALL) - ? fgAssignStructInlineeToVar(child, retClsHnd) // Assign to a variable if it is a call. - : child); // Just get the address, if not a call. - - tree->ReplaceWith(gtNewCpObjNode(dstAddr, srcAddr, retClsHnd, false), this); -} - -#endif // FEATURE_MULTIREG_RET - -//------------------------------------------------------------------------ -// fgUpdateInlineReturnExpressionPlaceHolder: callback to replace the -// inline return expression placeholder. -// -// Arguments: -// pTree -- pointer to tree to examine for updates -// data -- context data for the tree walk -// -// Returns: -// fgWalkResult indicating the walk should continue; that -// is we wish to fully explore the tree. -// -// Notes: -// Looks for GT_RET_EXPR nodes that arose from tree splitting done -// during importation for inline candidates, and replaces them. -// -// For successful inlines, substitutes the return value expression -// from the inline body for the GT_RET_EXPR. -// -// For failed inlines, rejoins the original call into the tree from -// whence it was split during importation. -// -// The code doesn't actually know if the corresponding inline -// succeeded or not; it relies on the fact that gtInlineCandidate -// initially points back at the call and is modified in place to -// the inlinee return expression if the inline is successful (see -// tail end of fgInsertInlineeBlocks for the update of iciCall). -// -// If the return type is a struct type and we're on a platform -// where structs can be returned in multiple registers, ensure the -// call has a suitable parent. -// -// If the original call type and the substitution type are different -// the functions makes necessary updates. It could happen if there was -// an implicit conversion in the inlinee body. -// -Compiler::fgWalkResult Compiler::fgUpdateInlineReturnExpressionPlaceHolder(GenTree** pTree, fgWalkData* data) -{ - // All the operations here and in the corresponding postorder - // callback (fgLateDevirtualization) are triggered by GT_CALL or - // GT_RET_EXPR trees, and these (should) have the call side - // effect flag. - // - // So bail out for any trees that don't have this flag. - GenTree* tree = *pTree; - - if ((tree->gtFlags & GTF_CALL) == 0) - { - return WALK_SKIP_SUBTREES; - } - - bool* madeChanges = static_cast(data->pCallbackData); - Compiler* comp = data->compiler; - CORINFO_CLASS_HANDLE retClsHnd = NO_CLASS_HANDLE; - - while (tree->OperIs(GT_RET_EXPR)) - { - // We are going to copy the tree from the inlinee, - // so record the handle now. - // - if (varTypeIsStruct(tree)) - { - retClsHnd = tree->AsRetExpr()->gtRetClsHnd; - } - - // Skip through chains of GT_RET_EXPRs (say from nested inlines) - // to the actual tree to use. - // - BasicBlockFlags bbFlags; - GenTree* inlineCandidate = tree; - do - { - GenTreeRetExpr* retExpr = inlineCandidate->AsRetExpr(); - inlineCandidate = retExpr->gtInlineCandidate; - bbFlags = retExpr->bbFlags; - } while (inlineCandidate->OperIs(GT_RET_EXPR)); - - // We might as well try and fold the return value. Eg returns of - // constant bools will have CASTS. This folding may uncover more - // GT_RET_EXPRs, so we loop around until we've got something distinct. - // - inlineCandidate = comp->gtFoldExpr(inlineCandidate); - var_types retType = tree->TypeGet(); - -#ifdef DEBUG - if (comp->verbose) - { - printf("\nReplacing the return expression placeholder "); - printTreeID(tree); - printf(" with "); - printTreeID(inlineCandidate); - printf("\n"); - // Dump out the old return expression placeholder it will be overwritten by the ReplaceWith below - comp->gtDispTree(tree); - } -#endif // DEBUG - - var_types newType = inlineCandidate->TypeGet(); - - // If we end up swapping type we may need to retype the tree: - if (retType != newType) - { - if ((retType == TYP_BYREF) && (tree->OperGet() == GT_IND)) - { - // - in an RVA static if we've reinterpreted it as a byref; - assert(newType == TYP_I_IMPL); - JITDUMP("Updating type of the return GT_IND expression to TYP_BYREF\n"); - inlineCandidate->gtType = TYP_BYREF; - } - } - - tree->ReplaceWith(inlineCandidate, comp); - *madeChanges = true; - comp->compCurBB->bbFlags |= (bbFlags & BBF_SPLIT_GAINED); - -#ifdef DEBUG - if (comp->verbose) - { - printf("\nInserting the inline return expression\n"); - comp->gtDispTree(tree); - printf("\n"); - } -#endif // DEBUG - } - - // If an inline was rejected and the call returns a struct, we may - // have deferred some work when importing call for cases where the - // struct is returned in register(s). - // - // See the bail-out clauses in impFixupCallStructReturn for inline - // candidates. - // - // Do the deferred work now. - if (retClsHnd != NO_CLASS_HANDLE) - { - structPassingKind howToReturnStruct; - var_types returnType = - comp->getReturnTypeForStruct(retClsHnd, CorInfoCallConvExtension::Managed, &howToReturnStruct); - GenTree* parent = data->parent; - - switch (howToReturnStruct) - { - -#if FEATURE_MULTIREG_RET - - // Is this a type that is returned in multiple registers - // or a via a primitve type that is larger than the struct type? - // if so we need to force into into a form we accept. - // i.e. LclVar = call() - case SPK_ByValue: - case SPK_ByValueAsHfa: - { - // See assert below, we only look one level above for an asg parent. - if (parent->gtOper == GT_ASG) - { - // Either lhs is a call V05 = call(); or lhs is addr, and asg becomes a copyBlk. - comp->fgAttachStructInlineeToAsg(parent, tree, retClsHnd); - } - else - { - // Just assign the inlinee to a variable to keep it simple. - tree->ReplaceWith(comp->fgAssignStructInlineeToVar(tree, retClsHnd), comp); - } - *madeChanges = true; - } - break; - -#endif // FEATURE_MULTIREG_RET - - case SPK_EnclosingType: - case SPK_PrimitiveType: - // No work needs to be done, the call has struct type and should keep it. - break; - - case SPK_ByReference: - // We should have already added the return buffer - // when we first imported the call - break; - - default: - noway_assert(!"Unexpected struct passing kind"); - break; - } - } - -#if FEATURE_MULTIREG_RET -#if defined(DEBUG) - - // Make sure we don't have a tree like so: V05 = (, , , retExpr); - // Since we only look one level above for the parent for '=' and - // do not check if there is a series of COMMAs. See above. - // Importer and FlowGraph will not generate such a tree, so just - // leaving an assert in here. This can be fixed by looking ahead - // when we visit GT_ASG similar to fgAttachStructInlineeToAsg. - // - if (tree->OperGet() == GT_ASG) - { - GenTree* value = tree->AsOp()->gtOp2; - - if (value->OperGet() == GT_COMMA) - { - GenTree* effectiveValue = value->gtEffectiveVal(/*commaOnly*/ true); - - noway_assert(!varTypeIsStruct(effectiveValue) || (effectiveValue->OperGet() != GT_RET_EXPR) || - !comp->IsMultiRegReturnedType(effectiveValue->AsRetExpr()->gtRetClsHnd, - CorInfoCallConvExtension::Managed)); - } - } - -#endif // defined(DEBUG) -#endif // FEATURE_MULTIREG_RET - - return WALK_CONTINUE; -} - -//------------------------------------------------------------------------ -// fgLateDevirtualization: re-examine calls after inlining to see if we -// can do more devirtualization -// -// Arguments: -// pTree -- pointer to tree to examine for updates -// data -- context data for the tree walk -// -// Returns: -// fgWalkResult indicating the walk should continue; that -// is we wish to fully explore the tree. -// -// Notes: -// We used to check this opportunistically in the preorder callback for -// calls where the `obj` was fed by a return, but we now re-examine -// all calls. -// -// Late devirtualization (and eventually, perhaps, other type-driven -// opts like cast optimization) can happen now because inlining or other -// optimizations may have provided more accurate types than we saw when -// first importing the trees. -// -// It would be nice to screen candidate sites based on the likelihood -// that something has changed. Otherwise we'll waste some time retrying -// an optimization that will just fail again. - -Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkData* data) -{ - GenTree* tree = *pTree; - GenTree* parent = data->parent; - Compiler* comp = data->compiler; - bool* madeChanges = static_cast(data->pCallbackData); - - // In some (rare) cases the parent node of tree will be smashed to a NOP during - // the preorder by fgAttachStructToInlineeArg. - // - // jit\Methodical\VT\callconv\_il_reljumper3 for x64 linux - // - // If so, just bail out here. - if (tree == nullptr) - { - assert((parent != nullptr) && parent->OperGet() == GT_NOP); - return WALK_CONTINUE; - } - - if (tree->OperGet() == GT_CALL) - { - GenTreeCall* call = tree->AsCall(); - bool tryLateDevirt = call->IsVirtual() && (call->gtCallType == CT_USER_FUNC); - -#ifdef DEBUG - tryLateDevirt = tryLateDevirt && (JitConfig.JitEnableLateDevirtualization() == 1); -#endif // DEBUG - - if (tryLateDevirt) - { -#ifdef DEBUG - if (comp->verbose) - { - printf("**** Late devirt opportunity\n"); - comp->gtDispTree(call); - } -#endif // DEBUG - - CORINFO_CONTEXT_HANDLE context = nullptr; - CORINFO_METHOD_HANDLE method = call->gtCallMethHnd; - unsigned methodFlags = 0; - const bool isLateDevirtualization = true; - const bool explicitTailCall = call->IsTailPrefixedCall(); - - if ((call->gtCallMoreFlags & GTF_CALL_M_HAS_LATE_DEVIRT_INFO) != 0) - { - context = call->gtLateDevirtualizationInfo->exactContextHnd; - // Note: we might call this multiple times for the same trees. - // If the devirtualization below succeeds, the call becomes - // non-virtual and we won't get here again. If it does not - // succeed we might get here again so we keep the late devirt - // info. - } - - comp->impDevirtualizeCall(call, nullptr, &method, &methodFlags, &context, nullptr, isLateDevirtualization, - explicitTailCall); - *madeChanges = true; - } - } - else if (tree->OperGet() == GT_ASG) - { - // If we're assigning to a ref typed local that has one definition, - // we may be able to sharpen the type for the local. - GenTree* const effLhs = tree->gtGetOp1()->gtEffectiveVal(); - - if ((effLhs->OperGet() == GT_LCL_VAR) && (effLhs->TypeGet() == TYP_REF)) - { - const unsigned lclNum = effLhs->AsLclVarCommon()->GetLclNum(); - LclVarDsc* lcl = comp->lvaGetDesc(lclNum); - - if (lcl->lvSingleDef) - { - GenTree* rhs = tree->gtGetOp2(); - bool isExact = false; - bool isNonNull = false; - CORINFO_CLASS_HANDLE newClass = comp->gtGetClassHandle(rhs, &isExact, &isNonNull); - - if (newClass != NO_CLASS_HANDLE) - { - comp->lvaUpdateClass(lclNum, newClass, isExact); - *madeChanges = true; - } - } - } - - // If we created a self-assignment (say because we are sharing return spill temps) - // we can remove it. - // - GenTree* const lhs = tree->gtGetOp1(); - GenTree* const rhs = tree->gtGetOp2(); - if (lhs->OperIs(GT_LCL_VAR) && GenTree::Compare(lhs, rhs)) - { - comp->gtUpdateNodeSideEffects(tree); - assert((tree->gtFlags & GTF_SIDE_EFFECT) == GTF_ASG); - JITDUMP("... removing self-assignment\n"); - DISPTREE(tree); - tree->gtBashToNOP(); - *madeChanges = true; - } - } - else if (tree->OperGet() == GT_JTRUE) - { - // See if this jtrue is now foldable. - BasicBlock* block = comp->compCurBB; - GenTree* condTree = tree->AsOp()->gtOp1; - assert(tree == block->lastStmt()->GetRootNode()); - - if (condTree->OperGet() == GT_CNS_INT) - { - JITDUMP(" ... found foldable jtrue at [%06u] in " FMT_BB "\n", dspTreeID(tree), block->bbNum); - noway_assert((block->bbNext->countOfInEdges() > 0) && (block->bbJumpDest->countOfInEdges() > 0)); - - // We have a constant operand, and should have the all clear to optimize. - // Update side effects on the tree, assert there aren't any, and bash to nop. - comp->gtUpdateNodeSideEffects(tree); - assert((tree->gtFlags & GTF_SIDE_EFFECT) == 0); - tree->gtBashToNOP(); - *madeChanges = true; - - BasicBlock* bNotTaken = nullptr; - - if (condTree->AsIntCon()->gtIconVal != 0) - { - block->bbJumpKind = BBJ_ALWAYS; - bNotTaken = block->bbNext; - } - else - { - block->bbJumpKind = BBJ_NONE; - bNotTaken = block->bbJumpDest; - } - - comp->fgRemoveRefPred(bNotTaken, block); - - // If that was the last ref, a subsequent flow-opt pass - // will clean up the now-unreachable bNotTaken, and any - // other transitively unreachable blocks. - if (bNotTaken->bbRefs == 0) - { - JITDUMP("... it looks like " FMT_BB " is now unreachable!\n", bNotTaken->bbNum); - } - } - } - else - { - const var_types retType = tree->TypeGet(); - GenTree* foldedTree = comp->gtFoldExpr(tree); - *pTree = foldedTree; - *madeChanges = true; - } - - return WALK_CONTINUE; -} - #ifdef DEBUG /***************************************************************************** diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 77b6bcf8f36ee..b9015e274a586 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -8978,7 +8978,64 @@ void Compiler::gtUpdateTreeAncestorsSideEffects(GenTree* tree) // void Compiler::gtUpdateStmtSideEffects(Statement* stmt) { - fgWalkTree(stmt->GetRootNodePointer(), fgUpdateSideEffectsPre, fgUpdateSideEffectsPost); + struct UpdateSideEffectsWalker : GenTreeVisitor + { + enum + { + DoPreOrder = true, + DoPostOrder = true, + }; + + UpdateSideEffectsWalker(Compiler* comp) : GenTreeVisitor(comp) + { + } + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* tree = *use; + tree->gtFlags &= ~(GTF_ASG | GTF_CALL | GTF_EXCEPT); + return WALK_CONTINUE; + } + + fgWalkResult PostOrderVisit(GenTree** use, GenTree* user) + { + GenTree* tree = *use; + + // Update the node's side effects first. + if (tree->OperMayThrow(m_compiler)) + { + tree->gtFlags |= GTF_EXCEPT; + } + + if (tree->OperRequiresAsgFlag()) + { + tree->gtFlags |= GTF_ASG; + } + + if (tree->OperRequiresCallFlag(m_compiler)) + { + tree->gtFlags |= GTF_CALL; + } + + // If this node is an indir or array meta-data load, and it doesn't have the GTF_EXCEPT bit set, we + // set the GTF_IND_NONFAULTING bit. This needs to be done after all children, and this node, have + // been processed. + if (tree->OperIsIndirOrArrMetaData() && ((tree->gtFlags & GTF_EXCEPT) == 0)) + { + tree->gtFlags |= GTF_IND_NONFAULTING; + } + + // Then update the parent's side effects based on this node. + if (user != nullptr) + { + user->gtFlags |= (tree->gtFlags & GTF_ALL_EFFECT); + } + return WALK_CONTINUE; + } + }; + + UpdateSideEffectsWalker walker(this); + walker.WalkTree(stmt->GetRootNodePointer(), nullptr); } //------------------------------------------------------------------------ @@ -9026,38 +9083,6 @@ void Compiler::gtUpdateNodeOperSideEffects(GenTree* tree) } } -//------------------------------------------------------------------------ -// gtUpdateNodeOperSideEffectsPost: Update the side effects based on the node operation, -// in the post-order visit of a tree walk. It is expected that the pre-order visit cleared -// the bits, so the post-order visit only sets them. This is important for binary nodes -// where one child already may have set the GTF_EXCEPT bit. Note that `SetIndirExceptionFlags` -// looks at its child, which is why we need to do this in a bottom-up walk. -// -// Arguments: -// tree - Tree to update the side effects on -// -// Notes: -// This method currently only updates GTF_ASG, GTF_CALL, and GTF_EXCEPT flags. -// The other side effect flags may remain unnecessarily (conservatively) set. -// -void Compiler::gtUpdateNodeOperSideEffectsPost(GenTree* tree) -{ - if (tree->OperMayThrow(this)) - { - tree->gtFlags |= GTF_EXCEPT; - } - - if (tree->OperRequiresAsgFlag()) - { - tree->gtFlags |= GTF_ASG; - } - - if (tree->OperRequiresCallFlag(this)) - { - tree->gtFlags |= GTF_CALL; - } -} - //------------------------------------------------------------------------ // gtUpdateNodeSideEffects: Update the side effects based on the node operation and // children's side efects. @@ -9078,58 +9103,6 @@ void Compiler::gtUpdateNodeSideEffects(GenTree* tree) }); } -//------------------------------------------------------------------------ -// fgUpdateSideEffectsPre: Update the side effects based on the tree operation. -// The pre-visit walk clears GTF_ASG, GTF_CALL, and GTF_EXCEPT; the post-visit walk sets -// the bits as necessary. -// -// Arguments: -// pTree - Pointer to the tree to update the side effects -// fgWalkPre - Walk data -// -Compiler::fgWalkResult Compiler::fgUpdateSideEffectsPre(GenTree** pTree, fgWalkData* fgWalkPre) -{ - GenTree* tree = *pTree; - tree->gtFlags &= ~(GTF_ASG | GTF_CALL | GTF_EXCEPT); - - return WALK_CONTINUE; -} - -//------------------------------------------------------------------------ -// fgUpdateSideEffectsPost: Update the side effects of the node and parent based on the tree's flags. -// -// Arguments: -// pTree - Pointer to the tree -// fgWalkPost - Walk data -// -// Notes: -// The routine is used for updating the stale side effect flags for ancestor -// nodes starting from treeParent up to the top-level stmt expr. -// -Compiler::fgWalkResult Compiler::fgUpdateSideEffectsPost(GenTree** pTree, fgWalkData* fgWalkPost) -{ - GenTree* tree = *pTree; - - // Update the node's side effects first. - fgWalkPost->compiler->gtUpdateNodeOperSideEffectsPost(tree); - - // If this node is an indir or array meta-data load, and it doesn't have the GTF_EXCEPT bit set, we - // set the GTF_IND_NONFAULTING bit. This needs to be done after all children, and this node, have - // been processed. - if (tree->OperIsIndirOrArrMetaData() && ((tree->gtFlags & GTF_EXCEPT) == 0)) - { - tree->gtFlags |= GTF_IND_NONFAULTING; - } - - // Then update the parent's side effects based on this node. - GenTree* parent = fgWalkPost->parent; - if (parent != nullptr) - { - parent->gtFlags |= (tree->gtFlags & GTF_ALL_EFFECT); - } - return WALK_CONTINUE; -} - bool GenTree::gtSetFlags() const { // @@ -16032,38 +16005,45 @@ Compiler::fgWalkResult Compiler::gtClearColonCond(GenTree** pTree, fgWalkData* d return WALK_CONTINUE; } -/***************************************************************************** - * - * Callback used by the tree walker to implement fgFindLink() - */ -static Compiler::fgWalkResult gtFindLinkCB(GenTree** pTree, Compiler::fgWalkData* cbData) +Compiler::FindLinkData Compiler::gtFindLink(Statement* stmt, GenTree* node) { - Compiler::FindLinkData* data = (Compiler::FindLinkData*)cbData->pCallbackData; - if (*pTree == data->nodeToFind) + class FindLinkWalker : public GenTreeVisitor { - data->result = pTree; - data->parent = cbData->parent; - return Compiler::WALK_ABORT; - } + GenTree* m_node; + GenTree** m_edge = nullptr; + GenTree* m_parent = nullptr; - return Compiler::WALK_CONTINUE; -} + public: + enum + { + DoPreOrder = true, + }; -Compiler::FindLinkData Compiler::gtFindLink(Statement* stmt, GenTree* node) -{ - FindLinkData data = {node, nullptr, nullptr}; + FindLinkWalker(Compiler* comp, GenTree* node) : GenTreeVisitor(comp), m_node(node) + { + } + + FindLinkData GetResult() + { + return FindLinkData{m_node, m_edge, m_parent}; + } - fgWalkResult result = fgWalkTreePre(stmt->GetRootNodePointer(), gtFindLinkCB, &data); + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + if (*use == m_node) + { + m_edge = use; + m_parent = user; + return WALK_ABORT; + } - if (result == WALK_ABORT) - { - assert(data.nodeToFind == *data.result); - return data; - } - else - { - return {node, nullptr, nullptr}; - } + return WALK_CONTINUE; + } + }; + + FindLinkWalker walker(this, node); + walker.WalkTree(stmt->GetRootNodePointer(), nullptr); + return walker.GetResult(); } /***************************************************************************** diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index d2cfa5b933f60..fe82d32776712 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -163,21 +163,6 @@ Compiler::fgWalkResult Compiler::optCSE_MaskHelper(GenTree** pTree, fgWalkData* Compiler* comp = walkData->compiler; optCSE_MaskData* pUserData = (optCSE_MaskData*)(walkData->pCallbackData); - if (IS_CSE_INDEX(tree->gtCSEnum)) - { - unsigned cseIndex = GET_CSE_INDEX(tree->gtCSEnum); - // Note that we DO NOT use getCSEAvailBit() here, for the CSE_defMask/CSE_useMask - unsigned cseBit = genCSEnum2bit(cseIndex); - if (IS_CSE_DEF(tree->gtCSEnum)) - { - BitVecOps::AddElemD(comp->cseMaskTraits, pUserData->CSE_defMask, cseBit); - } - else - { - BitVecOps::AddElemD(comp->cseMaskTraits, pUserData->CSE_useMask, cseBit); - } - } - return WALK_CONTINUE; } @@ -186,9 +171,45 @@ Compiler::fgWalkResult Compiler::optCSE_MaskHelper(GenTree** pTree, fgWalkData* // void Compiler::optCSE_GetMaskData(GenTree* tree, optCSE_MaskData* pMaskData) { + class MaskDataWalker : public GenTreeVisitor + { + optCSE_MaskData* m_maskData; + + public: + enum + { + DoPreOrder = true, + }; + + MaskDataWalker(Compiler* comp, optCSE_MaskData* maskData) : GenTreeVisitor(comp), m_maskData(maskData) + { + } + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* tree = *use; + if (IS_CSE_INDEX(tree->gtCSEnum)) + { + unsigned cseIndex = GET_CSE_INDEX(tree->gtCSEnum); + // Note that we DO NOT use getCSEAvailBit() here, for the CSE_defMask/CSE_useMask + unsigned cseBit = genCSEnum2bit(cseIndex); + if (IS_CSE_DEF(tree->gtCSEnum)) + { + BitVecOps::AddElemD(m_compiler->cseMaskTraits, m_maskData->CSE_defMask, cseBit); + } + else + { + BitVecOps::AddElemD(m_compiler->cseMaskTraits, m_maskData->CSE_useMask, cseBit); + } + } + return fgWalkResult::WALK_CONTINUE; + } + }; + pMaskData->CSE_defMask = BitVecOps::MakeEmpty(cseMaskTraits); pMaskData->CSE_useMask = BitVecOps::MakeEmpty(cseMaskTraits); - fgWalkTreePre(&tree, optCSE_MaskHelper, (void*)pMaskData); + MaskDataWalker walker(this, pMaskData); + walker.WalkTree(&tree, nullptr); } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index d796ac813d8c4..b3bee63ee71e6 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -4661,22 +4661,41 @@ bool Compiler::optReachWithoutCall(BasicBlock* topBB, BasicBlock* botBB) return true; } -// static -Compiler::fgWalkResult Compiler::optInvertCountTreeInfo(GenTree** pTree, fgWalkData* data) +Compiler::OptInvertCountTreeInfoType Compiler::optInvertCountTreeInfo(GenTree* tree) { - OptInvertCountTreeInfoType* o = (OptInvertCountTreeInfoType*)data->pCallbackData; - - if (Compiler::IsSharedStaticHelper(*pTree)) + class CountTreeInfoVisitor : public GenTreeVisitor { - o->sharedStaticHelperCount += 1; - } + public: + enum + { + DoPreOrder = true, + }; - if ((*pTree)->OperIsArrLength()) - { - o->arrayLengthCount += 1; - } + Compiler::OptInvertCountTreeInfoType Result = {}; - return WALK_CONTINUE; + CountTreeInfoVisitor(Compiler* comp) : GenTreeVisitor(comp) + { + } + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + if (Compiler::IsSharedStaticHelper(*use)) + { + Result.sharedStaticHelperCount++; + } + + if ((*use)->OperIsArrLength()) + { + Result.arrayLengthCount++; + } + + return fgWalkResult::WALK_CONTINUE; + } + }; + + CountTreeInfoVisitor walker(this); + walker.WalkTree(&tree, nullptr); + return walker.Result; } //----------------------------------------------------------------------------- @@ -4908,8 +4927,7 @@ bool Compiler::optInvertWhileLoop(BasicBlock* block) { GenTree* tree = stmt->GetRootNode(); - OptInvertCountTreeInfoType optInvertInfo = {}; - fgWalkTreePre(&tree, Compiler::optInvertCountTreeInfo, &optInvertInfo); + OptInvertCountTreeInfoType optInvertInfo = optInvertCountTreeInfo(tree); optInvertTotalInfo.sharedStaticHelperCount += optInvertInfo.sharedStaticHelperCount; optInvertTotalInfo.arrayLengthCount += optInvertInfo.arrayLengthCount; @@ -5960,97 +5978,114 @@ bool Compiler::optNarrowTree(GenTree* tree, var_types srct, var_types dstt, Valu } //------------------------------------------------------------------------ -// optIsVarAssgCB: callback to record local modification info +// optIsVarAssignedWithDesc: do a walk to record local modification data for a statement // // Arguments: -// pTree - pointer to tree to scan -// data - ambient walk data +// stmt - the statement to walk +// dsc - [in, out] data for the walk // -// Returns: -// standard walk result -// -Compiler::fgWalkResult Compiler::optIsVarAssgCB(GenTree** pTree, fgWalkData* data) +bool Compiler::optIsVarAssignedWithDesc(Statement* stmt, isVarAssgDsc* dsc) { - GenTree* const tree = *pTree; - isVarAssgDsc* const desc = (isVarAssgDsc*)data->pCallbackData; - assert(desc && desc->ivaSelf == desc); - - // Can this tree define a local? - // - if (!tree->OperIsSsaDef()) + class IsVarAssignedVisitor : public GenTreeVisitor { - return WALK_CONTINUE; - } + isVarAssgDsc* m_dsc; - // Check for calls and determine what's written. - // - GenTree* dest = nullptr; - if (tree->OperIs(GT_CALL)) - { - desc->ivaMaskCall = optCallInterf(tree->AsCall()); + public: + enum + { + DoPreOrder = true, + UseExecutionOrder = true, + }; - dest = data->compiler->gtCallGetDefinedRetBufLclAddr(tree->AsCall()); - if (dest == nullptr) + IsVarAssignedVisitor(Compiler* comp, isVarAssgDsc* dsc) : GenTreeVisitor(comp), m_dsc(dsc) { - return WALK_CONTINUE; } - dest = dest->AsOp()->gtOp1; - } - else - { - dest = tree->AsOp()->gtOp1; - } + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* const tree = *use; - genTreeOps const destOper = dest->OperGet(); + // Can this tree define a local? + // + if (!tree->OperIsSsaDef()) + { + return WALK_CONTINUE; + } - // Determine if the tree modifies a particular local - // - GenTreeLclVarCommon* lcl = nullptr; - if (tree->DefinesLocal(data->compiler, &lcl)) - { - const unsigned lclNum = lcl->GetLclNum(); + // Check for calls and determine what's written. + // + GenTree* dest = nullptr; + if (tree->OperIs(GT_CALL)) + { + m_dsc->ivaMaskCall = optCallInterf(tree->AsCall()); - if (lclNum < lclMAX_ALLSET_TRACKED) - { - AllVarSetOps::AddElemD(data->compiler, desc->ivaMaskVal, lclNum); - } - else - { - desc->ivaMaskIncomplete = true; - } + dest = m_compiler->gtCallGetDefinedRetBufLclAddr(tree->AsCall()); + if (dest == nullptr) + { + return WALK_CONTINUE; + } - // Bail out if we were checking for one particular local - // and we now see it's modified (ignoring perhaps - // the one tree where we expect modifications). - // - if ((lclNum == desc->ivaVar) && (tree != desc->ivaSkip)) - { - return WALK_ABORT; - } - } + dest = dest->AsOp()->gtOp1; + } + else + { + dest = tree->AsOp()->gtOp1; + } - if (destOper == GT_LCL_FLD) - { - // We can't track every field of every var. Moreover, indirections - // may access different parts of the var as different (but - // overlapping) fields. So just treat them as indirect accesses - // - // unsigned lclNum = dest->AsLclFld()->GetLclNum(); - // noway_assert(lvaTable[lclNum].lvAddrTaken); - // - varRefKinds refs = varTypeIsGC(tree->TypeGet()) ? VR_IND_REF : VR_IND_SCL; - desc->ivaMaskInd = varRefKinds(desc->ivaMaskInd | refs); - } - else if (destOper == GT_IND) - { - // Set the proper indirection bits - // - varRefKinds refs = varTypeIsGC(tree->TypeGet()) ? VR_IND_REF : VR_IND_SCL; - desc->ivaMaskInd = varRefKinds(desc->ivaMaskInd | refs); - } + genTreeOps const destOper = dest->OperGet(); - return WALK_CONTINUE; + // Determine if the tree modifies a particular local + // + GenTreeLclVarCommon* lcl = nullptr; + if (tree->DefinesLocal(m_compiler, &lcl)) + { + const unsigned lclNum = lcl->GetLclNum(); + + if (lclNum < lclMAX_ALLSET_TRACKED) + { + AllVarSetOps::AddElemD(m_compiler, m_dsc->ivaMaskVal, lclNum); + } + else + { + m_dsc->ivaMaskIncomplete = true; + } + + // Bail out if we were checking for one particular local + // and we now see it's modified (ignoring perhaps + // the one tree where we expect modifications). + // + if ((lclNum == m_dsc->ivaVar) && (tree != m_dsc->ivaSkip)) + { + return WALK_ABORT; + } + } + + if (destOper == GT_LCL_FLD) + { + // We can't track every field of every var. Moreover, indirections + // may access different parts of the var as different (but + // overlapping) fields. So just treat them as indirect accesses + // + // unsigned lclNum = dest->AsLclFld()->GetLclNum(); + // noway_assert(lvaTable[lclNum].lvAddrTaken); + // + varRefKinds refs = varTypeIsGC(tree->TypeGet()) ? VR_IND_REF : VR_IND_SCL; + m_dsc->ivaMaskInd = varRefKinds(m_dsc->ivaMaskInd | refs); + } + else if (destOper == GT_IND) + { + // Set the proper indirection bits + // + varRefKinds refs = varTypeIsGC(tree->TypeGet()) ? VR_IND_REF : VR_IND_SCL; + m_dsc->ivaMaskInd = varRefKinds(m_dsc->ivaMaskInd | refs); + } + + return WALK_CONTINUE; + } + }; + + IsVarAssignedVisitor walker(this, dsc); + return walker.WalkTree(stmt->GetRootNodePointer(), nullptr) != WALK_CONTINUE; } //------------------------------------------------------------------------ @@ -6072,13 +6107,9 @@ Compiler::fgWalkResult Compiler::optIsVarAssgCB(GenTree** pTree, fgWalkData* dat // bool Compiler::optIsVarAssigned(BasicBlock* beg, BasicBlock* end, GenTree* skip, unsigned var) { - bool result; isVarAssgDsc desc; - desc.ivaSkip = skip; -#ifdef DEBUG - desc.ivaSelf = &desc; -#endif + desc.ivaSkip = skip; desc.ivaVar = var; desc.ivaMaskCall = CALLINT_NONE; AllVarSetOps::AssignNoCopy(this, desc.ivaMaskVal, AllVarSetOps::MakeEmpty(this)); @@ -6089,10 +6120,9 @@ bool Compiler::optIsVarAssigned(BasicBlock* beg, BasicBlock* end, GenTree* skip, for (Statement* const stmt : beg->Statements()) { - if (fgWalkTreePre(stmt->GetRootNodePointer(), optIsVarAssgCB, &desc) != WALK_CONTINUE) + if (optIsVarAssignedWithDesc(stmt, &desc)) { - result = true; - goto DONE; + return true; } } @@ -6104,11 +6134,7 @@ bool Compiler::optIsVarAssigned(BasicBlock* beg, BasicBlock* end, GenTree* skip, beg = beg->bbNext; } - result = false; - -DONE: - - return result; + return false; } //------------------------------------------------------------------------ @@ -6204,9 +6230,6 @@ bool Compiler::optIsSetAssgLoop(unsigned lnum, ALLVARSET_VALARG_TP vars, varRefK // desc.ivaVar = (unsigned)-1; desc.ivaSkip = nullptr; -#ifdef DEBUG - desc.ivaSelf = &desc; -#endif AllVarSetOps::AssignNoCopy(this, desc.ivaMaskVal, AllVarSetOps::MakeEmpty(this)); desc.ivaMaskInd = VR_NONE; desc.ivaMaskCall = CALLINT_NONE; @@ -6218,7 +6241,7 @@ bool Compiler::optIsSetAssgLoop(unsigned lnum, ALLVARSET_VALARG_TP vars, varRefK { for (Statement* const stmt : block->NonPhiStatements()) { - fgWalkTreePre(stmt->GetRootNodePointer(), optIsVarAssgCB, &desc); + optIsVarAssignedWithDesc(stmt, &desc); if (desc.ivaMaskIncomplete) {