Codereview feedback
authorBrian Sullivan <briansul@microsoft.com>
Thu, 22 Feb 2018 01:13:41 +0000 (17:13 -0800)
committerBrian Sullivan <briansul@microsoft.com>
Thu, 22 Feb 2018 01:13:41 +0000 (17:13 -0800)
src/jit/optcse.cpp

index 9b534c9..bbfc27a 100644 (file)
@@ -2512,30 +2512,34 @@ void Compiler::optValnumCSE_Heuristic()
  *
  */
 
-bool Compiler::optValnumCSE_UnmarkCSEs(GenTree* deadTree, GenTree** wbKeepList)
+bool Compiler::optValnumCSE_UnmarkCSEs(GenTree* candidateTree, GenTree** wbKeepList)
 {
     Compiler::fgWalkResult result;
     assert(optValnumCSE_phase);
 
+    // We need to communicate the 'keepList' to both optHasCSEdefWithSideeffect
+    // and optUnmarkCSEs as any part of the 'candidateTree' tree that is in the
+    // keepList is preserved and is not deleted and does not have its ref counts
+    // decremented.
+    // We communicate this value using the walkData.pCallbackData field
+    //
+
     // First check for the rare case where we have a nested CSE def that has
     // side-effects and return false whenever we have this case
     //
-    result = fgWalkTreePre(&deadTree, optHasCSEdefWithSideeffect, (void*)wbKeepList);
-    if (result == WALK_ABORT)
+    Compiler::fgWalkResult defWithSideEffectResult = fgWalkTreePre(&candidateTree, optHasCSEdefWithSideeffect, (void*)wbKeepList);
+    bool hasPersistentSideEffect = (defWithSideEffectResult == WALK_ABORT);
+    if (hasPersistentSideEffect)
     {
         return false;
     }
+    else
+    {
+        Compiler::fgWalkResult unmarkResult = fgWalkTreePre(&candidateTree, optUnmarkCSEs, (void*)wbKeepList);
+        assert(unmarkResult != WALK_ABORT);
 
-    // We need to communicate the 'keepList' to optUnmarkCSEs
-    // as any part of the 'deadTree' tree that is in the keepList is preserved
-    // and is not deleted and does not have its ref counts decremented
-    // We communicate this value using the walkData.pCallbackData field
-    //
-
-    result = fgWalkTreePre(&deadTree, optUnmarkCSEs, (void*)wbKeepList);
-    assert(result != WALK_ABORT);
-
-    return true;
+        return true;
+    }
 }
 
 /*****************************************************************************