diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b748f3b81138a3..b678c0a4d383f0 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5594,7 +5594,7 @@ class Compiler void fgLiveVarAnalysis(); - GenTreeLclVarCommon* fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call); + bool fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call, GenTreeLclVarCommon** pDefinedLcl); void fgComputeLifeTrackedLocalUse(VARSET_TP& life, LclVarDsc& varDsc, GenTreeLclVarCommon* node); bool fgComputeLifeTrackedLocalDef(VARSET_TP& life, @@ -5622,12 +5622,9 @@ class Compiler bool fgTryRemoveDeadStoreLIR(GenTree* store, GenTreeLclVarCommon* lclNode, BasicBlock* block); - bool fgRemoveDeadStore(GenTree** pTree, - LclVarDsc* varDsc, - VARSET_VALARG_TP life, + bool fgRemoveDeadNode(GenTree** pTree, bool* doAgain, - bool* pStmtInfoDirty, - bool* pStoreRemoved DEBUGARG(bool* treeModf)); + bool* pStmtInfoDirty DEBUGARG(bool* treeModf)); void fgInterBlockLocalVarLiveness(); diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index e07afacc31dc44..f97053d060be66 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -771,14 +771,17 @@ void Compiler::fgLiveVarAnalysis() // life - The live set that is being computed. // keepAliveVars - Tracked locals that must be kept alive everywhere in the block // call - The call node in question. +// pDefinedLcl - [out] Local defined by the call, if any (eg retbuf) // // Returns: -// local defined by the call, if any (eg retbuf) +// True if the call's result is unused and the call can be removed // -GenTreeLclVarCommon* Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call) +bool Compiler::fgComputeLifeCall(VARSET_TP& life, + VARSET_VALARG_TP keepAliveVars, + GenTreeCall* call, + GenTreeLclVarCommon** pDefinedLcl) { assert(call != nullptr); - GenTreeLclVarCommon* definedLcl = nullptr; // If this is a tail-call via helper, and we have any unmanaged p/invoke calls in // the method, then we're going to run the p/invoke epilog @@ -838,13 +841,58 @@ GenTreeLclVarCommon* Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_ } } - definedLcl = gtCallGetDefinedRetBufLclAddr(call); + GenTreeLclVarCommon* definedLcl = gtCallGetDefinedRetBufLclAddr(call); if (definedLcl != nullptr) { fgComputeLifeLocal(life, keepAliveVars, definedLcl); } - return definedLcl; + if (pDefinedLcl != nullptr) + { + *pDefinedLcl = definedLcl; + } + + if (compRationalIRForm) + { + return (call->TypeIs(TYP_VOID) || call->IsUnusedValue()) && !call->HasSideEffects(this); + } + else + { + assert(fgNodeThreading == NodeThreading::AllTrees); + + if (!call->HasSideEffects(this)) + { + // See if the value of this call is used for anything. + GenTree* cur = call; + while (true) + { + if (cur->TypeIs(TYP_VOID)) + { + return true; + } + + GenTree* user = cur->gtGetParent(nullptr); + if (user == nullptr) + { + return true; + } + + if (!user->OperIs(GT_COMMA)) + { + return false; + } + + if (user->gtGetOp1() == cur) + { + return true; + } + + cur = user; + } + } + + return false; + } } //------------------------------------------------------------------------ @@ -1177,14 +1225,25 @@ void Compiler::fgComputeLife(VARSET_TP& life, AGAIN: assert(tree->OperGet() != GT_QMARK); - bool isUse = false; - bool doAgain = false; - bool storeRemoved = false; - LclVarDsc* varDsc = nullptr; + bool isUse = false; + bool doAgain = false; + LclVarDsc* varDsc = nullptr; if (tree->IsCall()) { - GenTreeLclVarCommon* const definedLcl = fgComputeLifeCall(life, keepAliveVars, tree->AsCall()); + GenTreeLclVarCommon* definedLcl; + bool isDeadCall = fgComputeLifeCall(life, keepAliveVars, tree->AsCall(), &definedLcl); + if (isDeadCall) + { + if (fgRemoveDeadNode(&tree, &doAgain, pStmtInfoDirty DEBUGARG(treeModf))) + { + assert(!doAgain); + break; + } + + definedLcl = nullptr; + } + if (definedLcl != nullptr) { isUse = (definedLcl->gtFlags & GTF_VAR_USEASG) != 0; @@ -1199,7 +1258,7 @@ void Compiler::fgComputeLife(VARSET_TP& life, { varDsc = lvaGetDesc(tree->AsLclVarCommon()); - if (fgRemoveDeadStore(&tree, varDsc, life, &doAgain, pStmtInfoDirty, &storeRemoved DEBUGARG(treeModf))) + if (fgRemoveDeadNode(&tree, &doAgain, pStmtInfoDirty DEBUGARG(treeModf))) { assert(!doAgain); break; @@ -1211,7 +1270,7 @@ void Compiler::fgComputeLife(VARSET_TP& life, // front-end liveness pass we must add them to the live set in case // we failed to remove the dead store. // - if ((varDsc != nullptr) && isUse && !storeRemoved) + if ((varDsc != nullptr) && isUse) { if (varDsc->lvTracked) { @@ -1265,8 +1324,9 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR { case GT_CALL: { - GenTreeCall* const call = node->AsCall(); - if (((call->TypeGet() == TYP_VOID) || call->IsUnusedValue()) && !call->HasSideEffects(this)) + GenTreeCall* const call = node->AsCall(); + bool isDeadCall = fgComputeLifeCall(life, keepAliveVars, call, nullptr); + if (isDeadCall) { JITDUMP("Removing dead call:\n"); DISPNODE(call); @@ -1300,10 +1360,7 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR fgStmtRemoved = true; } } - else - { - fgComputeLifeCall(life, keepAliveVars, call); - } + break; } @@ -1668,77 +1725,38 @@ bool Compiler::fgTryRemoveDeadStoreLIR(GenTree* store, GenTreeLclVarCommon* lclN // Return Value: // true if we should skip the rest of the statement, false if we should continue // -bool Compiler::fgRemoveDeadStore(GenTree** pTree, - LclVarDsc* varDsc, - VARSET_VALARG_TP life, - bool* doAgain, - bool* pStmtInfoDirty, - bool* pStoreRemoved DEBUGARG(bool* treeModf)) +bool Compiler::fgRemoveDeadNode(GenTree** pTree, bool* doAgain, bool* pStmtInfoDirty DEBUGARG(bool* treeModf)) { assert(!compRationalIRForm); - // Vars should have already been checked for address exposure by this point. - assert(!varDsc->IsAddressExposed()); - - GenTree* const tree = *pTree; - - // We can have two types of stores: STORE_LCL_VAR/STORE_LCL_FLD, ...) or a call, - // in which case we bail (we most likely cannot remove the call anyway). - if (!tree->OperIsLocalStore()) - { - *pStoreRemoved = false; - return false; - } - - // We are now committed to removing the store. - *pStoreRemoved = true; + GenTree* tree = *pTree; - GenTreeLclVarCommon* store = tree->AsLclVarCommon(); - GenTree* value = store->Data(); + JITDUMP("Removing dead node\n"); + DISPTREE(tree); // Check for side effects. GenTree* sideEffList = nullptr; - if ((value->gtFlags & GTF_SIDE_EFFECT) != 0) - { -#ifdef DEBUG - if (verbose) - { - printf(FMT_BB " - Dead store has side effects...\n", compCurBB->bbNum); - gtDispTree(store); - printf("\n"); - } -#endif // DEBUG - - gtExtractSideEffList(value, &sideEffList); - } + gtExtractSideEffList(tree, &sideEffList, GTF_SIDE_EFFECT, /* ignoreRoot */ true); // Test for interior statement - if (store->gtNext == nullptr) + if (tree->gtNext == nullptr) { // This is a "NORMAL" statement with the store node hanging from the statement. - noway_assert(compCurStmt->GetRootNode() == store); - JITDUMP("top level store\n"); + assert(compCurStmt->GetRootNode() == tree); + JITDUMP(" Top level node\n"); if (sideEffList != nullptr) { - noway_assert((sideEffList->gtFlags & GTF_SIDE_EFFECT) != 0); -#ifdef DEBUG - if (verbose) - { - printf("Extracted side effects list...\n"); - gtDispTree(sideEffList); - printf("\n"); - } -#endif // DEBUG - - // Replace the store statement with the list of side effects + JITDUMP(" Changing statement to side effects\n"); + // Replace the node with the list of side effects *pTree = sideEffList; compCurStmt->SetRootNode(sideEffList); -#ifdef DEBUG - *treeModf = true; -#endif // DEBUG - // Update ordering, costs, FP levels, etc. + INDEBUG(*treeModf = true); + + DISPSTMT(compCurStmt); + + // Update ordering, costs, FP levels, etc. gtSetStmtInfo(compCurStmt); // Re-link the nodes for this statement @@ -1754,7 +1772,7 @@ bool Compiler::fgRemoveDeadStore(GenTree** pTree, } else { - JITDUMP("removing stmt with no side effects\n"); + JITDUMP(" Removing stmt with no side effects\n"); // No side effects - remove the whole statement from the block->bbStmtList. fgRemoveStmt(compCurBB, compCurStmt); @@ -1768,76 +1786,37 @@ bool Compiler::fgRemoveDeadStore(GenTree** pTree, else { // This is an INTERIOR STATEMENT with a dead store - remove it. - // TODO-Cleanup: I'm not sure this assert is valuable; we've already determined this when - // we computed that it was dead. - if (varDsc->lvTracked) - { - noway_assert(!VarSetOps::IsMember(this, life, varDsc->lvVarIndex)); - } - else - { - for (unsigned i = 0; i < varDsc->lvFieldCnt; ++i) - { - unsigned fieldVarNum = varDsc->lvFieldLclStart + i; - { - LclVarDsc* fieldVarDsc = lvaGetDesc(fieldVarNum); - noway_assert(fieldVarDsc->lvTracked && !VarSetOps::IsMember(this, life, fieldVarDsc->lvVarIndex)); - } - } - } - + JITDUMP(" Interior node\n"); if (sideEffList != nullptr) { - noway_assert((sideEffList->gtFlags & GTF_SIDE_EFFECT) != 0); -#ifdef DEBUG - if (verbose) - { - printf("Extracted side effects list from condition...\n"); - gtDispTree(sideEffList); - printf("\n"); - } -#endif // DEBUG - -#ifdef DEBUG - *treeModf = true; -#endif // DEBUG + INDEBUG(*treeModf = true); // Change the node to a GT_COMMA holding the side effect list. - store->ChangeType(TYP_VOID); - store->ChangeOper(GT_COMMA); - store->SetAllEffectsFlags(sideEffList); + tree->ChangeType(TYP_VOID); + tree->ChangeOper(GT_COMMA); + tree->SetAllEffectsFlags(sideEffList); if (sideEffList->OperIs(GT_COMMA)) { - store->AsOp()->gtOp1 = sideEffList->AsOp()->gtOp1; - store->AsOp()->gtOp2 = sideEffList->AsOp()->gtOp2; + tree->AsOp()->gtOp1 = sideEffList->AsOp()->gtOp1; + tree->AsOp()->gtOp2 = sideEffList->AsOp()->gtOp2; } else { - store->AsOp()->gtOp1 = sideEffList; - store->AsOp()->gtOp2 = gtNewNothingNode(); + tree->AsOp()->gtOp1 = sideEffList; + tree->AsOp()->gtOp2 = gtNewNothingNode(); } } else { -#ifdef DEBUG - if (verbose) - { - printf("\nRemoving tree "); - printTreeID(store); - printf(" in " FMT_BB " as useless\n", compCurBB->bbNum); - gtDispTree(store); - printf("\n"); - } -#endif // DEBUG - // No side effects - Change the store to a GT_NOP node - store->gtBashToNOP(); + // No side effects - Change the store to a GT_NOP node + tree->gtBashToNOP(); -#ifdef DEBUG - *treeModf = true; -#endif // DEBUG + INDEBUG(*treeModf = true); } + DISPTREE(tree); + // Re-link the nodes for this statement - Do not update ordering! // Do not update costs by calling gtSetStmtInfo. fgSetStmtSeq modifies @@ -1848,14 +1827,8 @@ bool Compiler::fgRemoveDeadStore(GenTree** pTree, *pStmtInfoDirty = true; fgSetStmtSeq(compCurStmt); - - // Continue analysis from this node - *pTree = store; - return false; } - - return false; } /*****************************************************************************