Transform dead late arg store to NOP (#40348)
authorCarol Eidt <carol.eidt@microsoft.com>
Wed, 5 Aug 2020 18:33:30 +0000 (11:33 -0700)
committerGitHub <noreply@github.com>
Wed, 5 Aug 2020 18:33:30 +0000 (11:33 -0700)
When deleting a dead store, preserve the GTF_LATE_ARG and transform it to NOP.

Fix #39742

src/coreclr/src/jit/compiler.h
src/coreclr/src/jit/liveness.cpp
src/coreclr/src/jit/lower.cpp

index 23c3b68..9924c5f 100644 (file)
@@ -4637,6 +4637,7 @@ public:
 
     bool fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange);
 
+    void fgRemoveDeadStoreLIR(GenTree* store, BasicBlock* block);
     bool fgRemoveDeadStore(GenTree**        pTree,
                            LclVarDsc*       varDsc,
                            VARSET_VALARG_TP life,
index 5c3c860..cbc4ebb 100644 (file)
@@ -1986,10 +1986,7 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
                                 Lowering::TransformUnusedIndirection(data->AsIndir(), this, block);
                             }
 
-                            blockRange.Remove(store);
-
-                            assert(!opts.MinOpts());
-                            fgStmtRemoved = true;
+                            fgRemoveDeadStoreLIR(store, block);
                         }
                     }
                 }
@@ -2018,25 +2015,7 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
                     // Remove the store. DCE will iteratively clean up any ununsed operands.
                     lclVarNode->gtOp1->SetUnusedValue();
 
-                    // If the store is marked as a late argument, it is referenced by a call. Instead of removing
-                    // it, bash it to a NOP.
-                    if ((node->gtFlags & GTF_LATE_ARG) != 0)
-                    {
-                        JITDUMP("node is a late arg; replacing with NOP\n");
-                        node->gtBashToNOP();
-
-                        // NOTE: this is a bit of a hack. We need to keep these nodes around as they are
-                        // referenced by the call, but they're considered side-effect-free non-value-producing
-                        // nodes, so they will be removed if we don't do this.
-                        node->gtFlags |= GTF_ORDER_SIDEEFF;
-                    }
-                    else
-                    {
-                        blockRange.Remove(node);
-                    }
-
-                    assert(!opts.MinOpts());
-                    fgStmtRemoved = true;
+                    fgRemoveDeadStoreLIR(node, block);
                 }
 
                 break;
@@ -2179,6 +2158,41 @@ bool Compiler::fgTryRemoveNonLocal(GenTree* node, LIR::Range* blockRange)
     return false;
 }
 
+//---------------------------------------------------------------------
+// fgRemoveDeadStoreSimple - remove a dead store
+//
+//   pTree          - GenTree** to local, including store-form local or local addr (post-rationalize)
+//   varDsc         - var that is being stored to
+//   life           - current live tracked vars (maintained as we walk backwards)
+//   doAgain        - out parameter, true if we should restart the statement
+//   pStmtInfoDirty - should defer the cost computation to the point after the reverse walk is completed?
+//
+void Compiler::fgRemoveDeadStoreLIR(GenTree* store, BasicBlock* block)
+{
+    LIR::Range& blockRange = LIR::AsRange(block);
+
+    // If the store is marked as a late argument, it is referenced by a call.
+    // Instead of removing it, bash it to a NOP.
+    if ((store->gtFlags & GTF_LATE_ARG) != 0)
+    {
+        JITDUMP("node is a late arg; replacing with NOP\n");
+        store->gtBashToNOP();
+
+        // NOTE: this is a bit of a hack. We need to keep these nodes around as they are
+        // referenced by the call, but they're considered side-effect-free non-value-producing
+        // nodes, so they will be removed if we don't do this.
+        store->gtFlags |= GTF_ORDER_SIDEEFF;
+    }
+    else
+    {
+        blockRange.Remove(store);
+    }
+
+    assert(!opts.MinOpts());
+    fgStmtRemoved = true;
+}
+
+//---------------------------------------------------------------------
 // fgRemoveDeadStore - remove a store to a local which has no exposed uses.
 //
 //   pTree          - GenTree** to local, including store-form local or local addr (post-rationalize)
index 61e15a3..a401a84 100644 (file)
@@ -3181,7 +3181,9 @@ void Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore)
             // Create the assignment node.
             lclStore->ChangeOper(GT_STORE_OBJ);
             GenTreeBlk* objStore = lclStore->AsObj();
-            objStore->gtFlags    = GTF_ASG | GTF_IND_NONFAULTING | GTF_IND_TGT_NOT_HEAP;
+            // Only the GTF_LATE_ARG flag (if present) is preserved.
+            objStore->gtFlags &= GTF_LATE_ARG;
+            objStore->gtFlags |= GTF_ASG | GTF_IND_NONFAULTING | GTF_IND_TGT_NOT_HEAP;
 #ifndef JIT32_GCENCODER
             objStore->gtBlkOpGcUnsafe = false;
 #endif