JIT: remove unneeded ref count updating traversal from optimizer (#22954)
authorAndy Ayers <andya@microsoft.com>
Tue, 5 Mar 2019 07:30:48 +0000 (23:30 -0800)
committerGitHub <noreply@github.com>
Tue, 5 Mar 2019 07:30:48 +0000 (23:30 -0800)
The ref count update traversal in the optimizer is not doing anything,
so remove it. This was overlooked when we changed away from incremental
updates in #19345.

Also: fix up comments and documentation to reflect the current approach
to local var ref counts.

Documentation/botr/ryujit-overview.md
src/jit/assertionprop.cpp
src/jit/compiler.h
src/jit/morph.cpp
src/jit/optcse.cpp
src/jit/optimizer.cpp

index bf8b2d4..fad15f4 100644 (file)
@@ -195,7 +195,9 @@ At this point, a number of analyses and transformations are done on the flowgrap
 
 At this point, a number of properties are computed on the IR, and must remain valid for the remaining phases. We will call this “normalization”
 
-* `lvaMarkLocalVars` – set the reference counts (raw and weighted) for lclVars, sort them, and determine which will be tracked (currently up to 128). Note that after this point any transformation that adds or removes lclVar references must update the reference counts.
+* `lvaMarkLocalVars` – if this jit is optimizing, set the reference counts (raw and weighted) for lclVars, sort them, and determine which
+will be tracked (currently up to 512). If not optimizing, all locals are given an implicit reference count of one. Reference counts are not incrementally
+maintained. They can be recomputed if accurate counts are needed.
 * `optOptimizeBools` – this optimizes Boolean expressions, and may change the flowgraph (why is it not done prior to reachability and dominators?)
 * Link the trees in evaluation order (setting `gtNext` and `gtPrev` fields): and `fgFindOperOrder()` and `fgSetBlockOrder()`.
 
@@ -492,7 +494,16 @@ TreeNodeInfo:
 
 ## LclVar phase-dependent properties
 
-Prior to normalization, the reference counts (`lvRefCnt` and `lvRefCntWtd`) are not valid. After normalization they must be updated when lclVar references are added or removed.
+LclVar ref counts track the number of uses and weighted used of a local in the jit IR. There are two sequences of phases over which ref counts are valid,
+tracked via `lvaRefCountState`: an early sequence (state `RCS_EARLY`) and the normal sequence (state `RCS_NORMAL`). Requests for ref counts via `lvRefCnt`
+and `lvRefCntWtd` must be aware of the ref count state.
+
+Before struct promotion the ref counts are invalid. Struct promotion enables `RCS_EARLY` and it and subsequent phases through morph compute and uses ref counts
+on some locals to guide some struct optimizations. After morph the counts go back to longer being valid.
+
+The `RCS_NORMAL` sequence begins at normalization. Ref counts are computed and generally available via for the rest of the compilation phases.
+The counts are not incrementally maintained and may go stale as the IR is optimized or transformed, or maybe very approximate if the jit is not optimizing.
+They can be recomputed via `lvaComputeRefCounts` at points where accurate counts are valuable. Currently this happens before and after lower.
 
 # Supporting technologies and components
 
index 539374f..0ec9f4b 100644 (file)
@@ -4651,8 +4651,8 @@ struct VNAssertionPropVisitorInfo
 
 //------------------------------------------------------------------------------
 // optPrepareTreeForReplacement
-//    Updates ref counts and extracts side effects from a tree so it can be
-//    replaced with a comma separated list of side effects + a new tree.
+//    Extracts side effects from a tree so it can be replaced with a comma
+//    separated list of side effects + a new tree.
 //
 // Note:
 //    The old and new trees may be the same. In this case, the tree will be
@@ -4674,10 +4674,6 @@ struct VNAssertionPropVisitorInfo
 //      2. When no side-effects are present, returns null.
 //
 // Description:
-//    Decrements ref counts for the "oldTree" that is going to be replaced. If there
-//    are side effects in the tree, then ref counts for variables in the side effects
-//    are incremented because they need to be kept in the stmt expr.
-//
 //    Either the "newTree" is returned when no side effects are present or a comma
 //    separated side effect list with "newTree" is returned.
 //
index 5fec254..fbf7fdc 100644 (file)
@@ -5473,10 +5473,6 @@ public:
 
 protected:
     static fgWalkPreFn optValidRangeCheckIndex;
-    static fgWalkPreFn optRemoveTreeVisitor; // Helper passed to Compiler::fgWalkAllTreesPre() to decrement the LclVar
-                                             // usage counts
-
-    void optRemoveTree(GenTree* deadTree, GenTree* keepList);
 
     /**************************************************************************
      *
index c26134c..48d97cf 100644 (file)
@@ -2517,8 +2517,7 @@ int Compiler::fgEstimateCallStackSize(GenTreeCall* call)
 }
 
 //------------------------------------------------------------------------------
-// fgMakeMultiUse : If the node is a local, clone it and increase the ref count
-//                  otherwise insert a comma form temp
+// fgMakeMultiUse : If the node is a local, clone it, otherwise insert a comma form temp
 //
 // Arguments:
 //    ppTree  - a pointer to the child node we will be replacing with the comma expression that
@@ -2526,10 +2525,6 @@ int Compiler::fgEstimateCallStackSize(GenTreeCall* call)
 //
 // Return Value:
 //    A fresh GT_LCL_VAR node referencing the temp which has not been used
-//
-// Assumption:
-//    The result tree MUST be added to the tree structure since the ref counts are
-//    already incremented.
 
 GenTree* Compiler::fgMakeMultiUse(GenTree** pOp)
 {
index 648e670..8ef5ae7 100644 (file)
@@ -2225,7 +2225,7 @@ public:
                 // these are appended to the sideEffList
 
                 // Afterwards the set of nodes in the 'sideEffectList' are preserved and
-                // all other nodes are removed and have their ref counts decremented
+                // all other nodes are removed.
                 //
                 exp->gtCSEnum = NO_CSE; // clear the gtCSEnum field
 
index bf9140a..9d9b515 100644 (file)
@@ -7895,74 +7895,6 @@ void Compiler::AddModifiedElemTypeAllContainingLoops(unsigned lnum, CORINFO_CLAS
     }
 }
 
-/*****************************************************************************
- *
- *  Helper passed to Compiler::fgWalkAllTreesPre() to decrement the LclVar usage counts
- *  The 'keepList'is either a single tree or a list of trees that are formed by
- *  one or more GT_COMMA nodes.  It is the kept side-effects as returned by the
- *  gtExtractSideEffList method.
- */
-
-/* static */
-Compiler::fgWalkResult Compiler::optRemoveTreeVisitor(GenTree** pTree, fgWalkData* data)
-{
-    GenTree*  tree     = *pTree;
-    Compiler* comp     = data->compiler;
-    GenTree*  keepList = (GenTree*)(data->pCallbackData);
-
-    // We may have a non-NULL side effect list that is being kept
-    //
-    if (keepList)
-    {
-        GenTree* keptTree = keepList;
-        while (keptTree->OperGet() == GT_COMMA)
-        {
-            assert(keptTree->OperKind() & GTK_SMPOP);
-            GenTree* op1 = keptTree->gtOp.gtOp1;
-            GenTree* op2 = keptTree->gtGetOp2();
-
-            // For the GT_COMMA case the op1 is part of the orginal CSE tree
-            // that is being kept because it contains some side-effect
-            //
-            if (tree == op1)
-            {
-                // This tree and all of its sub trees are being kept.
-                return WALK_SKIP_SUBTREES;
-            }
-
-            // For the GT_COMMA case the op2 are the remaining side-effects of the orginal CSE tree
-            // which can again be another GT_COMMA or the final side-effect part
-            //
-            keptTree = op2;
-        }
-        if (tree == keptTree)
-        {
-            // This tree and all of its sub trees are being kept.
-            return WALK_SKIP_SUBTREES;
-        }
-    }
-
-    return WALK_CONTINUE;
-}
-
-/*****************************************************************************
- *
- *  Routine called to decrement the LclVar ref counts when removing a tree
- *  during the remove RangeCheck phase.
- *  This method will decrement the refcounts for any LclVars used below 'deadTree',
- *  unless the node is found in the 'keepList' (which are saved side effects)
- *  The keepList is communicated using the walkData.pCallbackData field
- *  Also the compCurBB must be set to the current BasicBlock  which contains
- *  'deadTree' as we need to fetch the block weight when decrementing the ref counts.
- */
-
-void Compiler::optRemoveTree(GenTree* deadTree, GenTree* keepList)
-{
-    // We communicate this value using the walkData.pCallbackData field
-    //
-    fgWalkTreePre(&deadTree, optRemoveTreeVisitor, (void*)keepList);
-}
-
 //------------------------------------------------------------------------------
 // optRemoveRangeCheck : Given an array index node, mark it as not needing a range check.
 //
@@ -7993,14 +7925,10 @@ void Compiler::optRemoveRangeCheck(GenTree* tree, GenTree* stmt)
     }
 #endif
 
+    // Extract side effects
     GenTree* sideEffList = nullptr;
-
     gtExtractSideEffList(bndsChkTree, &sideEffList, GTF_ASG);
 
-    // Decrement the ref counts for any LclVars that are being deleted
-    //
-    optRemoveTree(bndsChkTree, sideEffList);
-
     // Just replace the bndsChk with a NOP as an operand to the GT_COMMA, if there are no side effects.
     tree->gtOp.gtOp1 = (sideEffList != nullptr) ? sideEffList : gtNewNothingNode();
     // TODO-CQ: We should also remove the GT_COMMA, but in any case we can no longer CSE the GT_COMMA.