Fix for assert(sideEffList) in clr\src\jit\optcse.cpp, Line: 2151
authorBrian Sullivan <briansul@microsoft.com>
Fri, 16 Feb 2018 01:15:59 +0000 (17:15 -0800)
committerBrian Sullivan <briansul@microsoft.com>
Fri, 16 Feb 2018 01:39:21 +0000 (17:39 -0800)
If we return WALK_ABORT this assert will fire when the side effect list 'keepList' is empty.
With this fix we don't return WALK_ABORT when the keepList is empty, instead we just place
the CSE def into the keepList

src/jit/optcse.cpp

index 1ce5442c219e208e54132fc4557a01c9ef8507ac..043919e5f6cf6257f45588c22fd97691d72e9052 100644 (file)
@@ -312,10 +312,17 @@ Compiler::fgWalkResult Compiler::optUnmarkCSEs(GenTree** pTree, fgWalkData* data
         // Instead we will add it to the 'keepList'.
         assert(IS_CSE_DEF(tree->gtCSEnum));
 
-        if (comp->gtTreeHasSideEffects(tree, GTF_PERSISTENT_SIDE_EFFECTS_IN_CSE))
+        // If we already have collected any persistent side effects then keepList is non-null
+        //  and we will return WALK_ABORT since we have a CSE def that we must keep.
+        //
+        // If we haven't kept any  side effects yet, we will create new GT_COMMA list with this one.
+        //
+        if ((keepList != nullptr) && comp->gtTreeHasSideEffects(tree, GTF_PERSISTENT_SIDE_EFFECTS_IN_CSE))
         {
-            // If the nested CSE def has persistent side effects then just abort
-            // as this case is problematic.
+            // If this nested CSE def has a persistent side effect and
+            //  we are already keeping other side effects then
+            //  just abort as this case is problematic.
+            // a
             return WALK_ABORT;
         }
         else