diff options
author | Matt Mitchell <mmitche@microsoft.com> | 2015-07-29 21:44:22 -0700 |
---|---|---|
committer | Matt Mitchell <mmitche@microsoft.com> | 2015-07-29 21:44:22 -0700 |
commit | b53dd122d5c3fa4df4dcb0f65eb0ec57d87551a8 (patch) | |
tree | c509c8564730c30859a07bfad05004add99c74a4 | |
parent | f5794741fdcacc227db3405f462a44a681c6d0d2 (diff) | |
parent | e4ae6720d1a09499dfc71a8df4b308502bc2fea3 (diff) | |
download | coreclr-b53dd122d5c3fa4df4dcb0f65eb0ec57d87551a8.tar.gz coreclr-b53dd122d5c3fa4df4dcb0f65eb0ec57d87551a8.tar.bz2 coreclr-b53dd122d5c3fa4df4dcb0f65eb0ec57d87551a8.zip |
Merge pull request #1309 from schellap/copyprop
Conservatively mark variable as killed if there is an embedded comma …
-rw-r--r-- | src/jit/compiler.h | 4 | ||||
-rw-r--r-- | src/jit/copyprop.cpp | 48 |
2 files changed, 46 insertions, 6 deletions
diff --git a/src/jit/compiler.h b/src/jit/compiler.h index e1b8e9bbd9..c6063dae20 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -5302,10 +5302,14 @@ public: typedef ArrayStack<GenTreePtr> GenTreePtrStack; typedef SimplerHashTable<unsigned, SmallPrimitiveKeyFuncs<unsigned>, GenTreePtrStack*, DefaultSimplerHashBehavior> LclNumToGenTreePtrStack; + // Kill set to track variables with intervening definitions. + VARSET_TP optCopyPropKillSet; + // Copy propagation functions. void optCopyProp(BasicBlock* block, GenTreePtr stmt, GenTreePtr tree, LclNumToGenTreePtrStack* curSsaName); void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName); void optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curSsaName); + bool optIsSsaLocal(GenTreePtr tree); int optCopyProp_LclVarScore(LclVarDsc* lclVarDsc, LclVarDsc* copyVarDsc, bool preferOp2); void optVnCopyProp(); diff --git a/src/jit/copyprop.cpp b/src/jit/copyprop.cpp index 1ded34950a..a4c56a1c46 100644 --- a/src/jit/copyprop.cpp +++ b/src/jit/copyprop.cpp @@ -165,6 +165,14 @@ void Compiler::optCopyProp(BasicBlock* block, GenTreePtr stmt, GenTreePtr tree, { continue; } + + // Skip variables with assignments embedded in the statement (i.e., with a comma). Because we + // are not currently updating their SSA names as live in the copy-prop pass of the stmt. + if (VarSetOps::IsMember(this, optCopyPropKillSet, lvaTable[newLclNum].lvVarIndex)) + { + continue; + } + if (op->gtFlags & GTF_VAR_CAST) { continue; @@ -268,6 +276,15 @@ void Compiler::optCopyProp(BasicBlock* block, GenTreePtr stmt, GenTreePtr tree, /************************************************************************************** * + * Helper to check if tree is a local that participates in SSA numbering. + */ +bool Compiler::optIsSsaLocal(GenTreePtr tree) +{ + return tree->IsLocal() && !fgExcludeFromSsa(tree->AsLclVarCommon()->GetLclNum()); +} + +/************************************************************************************** + * * Perform copy propagation using currently live definitions on the current block's * variables. Also as new definitions are encountered update the "curSsaName" which * tracks the currently live definitions. @@ -282,26 +299,44 @@ void Compiler::optBlockCopyProp(BasicBlock* block, LclNumToGenTreePtrStack* curS VarSetOps::Assign(this, compCurLife, block->bbLiveIn); for (GenTreePtr stmt = block->bbTreeList; stmt; stmt = stmt->gtNext) { + VarSetOps::ClearD(this, optCopyPropKillSet); + // Walk the tree to find if any local variable can be replaced with current live definitions. for (GenTreePtr tree = stmt->gtStmt.gtStmtList; tree; tree = tree->gtNext) { compUpdateLife</*ForCodeGen*/false>(tree); optCopyProp(block, stmt, tree, curSsaName); + + // TODO-Review: Merge this loop with the following loop to correctly update the + // live SSA num while also propagating copies. + // + // 1. This loop performs copy prop with currently live (on-top-of-stack) SSA num. + // 2. The subsequent loop maintains a stack for each lclNum with + // currently active SSA numbers when definitions are encountered. + // + // If there is an embedded definition using a "comma" in a stmt, then the currently + // live SSA number will get updated only in the next loop (2). However, this new + // definition is now supposed to be live (on tos). If we did not update the stacks + // using (2), copy prop (1) will use a SSA num defined outside the stmt ignoring the + // embedded update. Killing the variable is a simplification to produce 0 ASM diffs + // for an update release. + // + if (optIsSsaLocal(tree) && (tree->gtFlags & GTF_VAR_DEF)) + { + VarSetOps::AddElemD(this, optCopyPropKillSet, lvaTable[tree->gtLclVarCommon.gtLclNum].lvVarIndex); + } } // This logic must be in sync with SSA renaming process. for (GenTreePtr tree = stmt->gtStmt.gtStmtList; tree; tree = tree->gtNext) { - if (!tree->IsLocal()) + if (!optIsSsaLocal(tree)) { continue; } - unsigned lclNum = tree->AsLclVarCommon()->GetLclNum(); - if (fgExcludeFromSsa(lclNum)) - { - continue; - } + unsigned lclNum = tree->gtLclVarCommon.gtLclNum; + // As we encounter a definition add it to the stack as a live definition. if (tree->gtFlags & GTF_VAR_DEF) { @@ -387,6 +422,7 @@ void Compiler::optVnCopyProp() typedef jitstd::vector<BlockWork> BlockWorkStack; VarSetOps::AssignNoCopy(this, compCurLife, VarSetOps::MakeEmpty(this)); + VarSetOps::AssignNoCopy(this, optCopyPropKillSet, VarSetOps::MakeEmpty(this)); // The map from lclNum to its recently live definitions as a stack. LclNumToGenTreePtrStack curSsaName(getAllocator()); |