Mark operands of dead FIELD_LIST as unused

This requires fixing the side-effects check in dead code elimination.
Also, fixes gtSetFlags() to be usable from DCE in the non-legacy case.

6 files changed, 30 insertions, 7 deletions

diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp
index 9b8f88bd85..937efef35b 100644
--- a/src/jit/gentree.cpp
+++ b/src/jit/gentree.cpp
@@ -8699,7 +8699,7 @@ bool GenTree::gtSetFlags() const
     //
     // Precondition we have a GTK_SMPOP
     //
-    if (!varTypeIsIntegralOrI(TypeGet()))
+    if (!varTypeIsIntegralOrI(TypeGet()) && (TypeGet() != TYP_VOID))
     {
         return false;
     }
@@ -8722,7 +8722,7 @@ bool GenTree::gtSetFlags() const
     }
 #else // !(defined(LEGACY_BACKEND) && !FEATURE_SET_FLAGS && defined(_TARGET_XARCH_))
 
-#if FEATURE_SET_FLAGS
+#if FEATURE_SET_FLAGS && defined(LEGACY_BACKEND)
     assert(OperIsSimple());
 #endif
     if (((gtFlags & GTF_SET_FLAGS) != 0) && (gtOper != GT_IND))
diff --git a/src/jit/lir.cpp b/src/jit/lir.cpp
index a0a265d5ae..5a05e23c8e 100644
--- a/src/jit/lir.cpp
+++ b/src/jit/lir.cpp
@@ -1581,6 +1581,11 @@ bool LIR::Range::CheckLIR(Compiler* compiler, bool checkUnusedValues) const
         // Verify that the node is allowed in LIR.
         assert(node->IsLIR());
 
+        // Some nodes should never be marked unused, as they must be contained in the backend.
+        // These may be marked as unused during dead code elimination traversal, but they *must* be subsequently
+        // removed.
+        assert(!node->IsUnusedValue() || !node->OperIs(GT_FIELD_LIST, GT_LIST, GT_INIT_VAL));
+
         // Verify that the REVERSE_OPS flag is not set. NOTE: if we ever decide to reuse the bit assigned to
         // GTF_REVERSE_OPS for an LIR-only flag we will need to move this check to the points at which we
         // insert nodes into an LIR range.
diff --git a/src/jit/liveness.cpp b/src/jit/liveness.cpp
index dac5a00e1b..67d9e827a9 100644
--- a/src/jit/liveness.cpp
+++ b/src/jit/liveness.cpp
@@ -2225,8 +2225,11 @@ void Compiler::fgComputeLifeLIR(VARSET_TP& life, BasicBlock* block, VARSET_VALAR
                 assert(!node->OperIsLocal());
                 if (!node->IsValue() || node->IsUnusedValue())
                 {
-                    unsigned sideEffects = node->gtFlags & (GTF_SIDE_EFFECT | GTF_SET_FLAGS);
-                    if ((sideEffects == 0) || ((sideEffects == GTF_EXCEPT) && !node->OperMayThrow(this)))
+                    // We are only interested in avoiding the removal of nodes with direct side-effects
+                    // (as opposed to side effects of their children).
+                    // This default case should never include calls or assignments.
+                    assert(!node->OperRequiresAsgFlag() && !node->OperIs(GT_CALL));
+                    if (!node->gtSetFlags() && !node->OperMayThrow(this))
                     {
                         JITDUMP("Removing dead node:\n");
                         DISPNODE(node);
diff --git a/src/jit/lsraarm.cpp b/src/jit/lsraarm.cpp
index fb3df19139..df4a435589 100644
--- a/src/jit/lsraarm.cpp
+++ b/src/jit/lsraarm.cpp
@@ -373,8 +373,13 @@ void LinearScan::BuildNode(GenTree* tree)
             assert(info->srcCount == 2);
             break;
 
-        case GT_LIST:
         case GT_FIELD_LIST:
+            // These should always be contained. We don't correctly allocate or
+            // generate code for a non-contained GT_FIELD_LIST.
+            noway_assert(!"Non-contained GT_FIELD_LIST");
+            break;
+
+        case GT_LIST:
         case GT_ARGPLACE:
         case GT_NO_OP:
         case GT_START_NONGC:
diff --git a/src/jit/lsraarm64.cpp b/src/jit/lsraarm64.cpp
index 1f367915d1..6497ac877a 100644
--- a/src/jit/lsraarm64.cpp
+++ b/src/jit/lsraarm64.cpp
@@ -87,8 +87,13 @@ void LinearScan::BuildNode(GenTree* tree)
             BuildStoreLoc(tree->AsLclVarCommon());
             break;
 
-        case GT_LIST:
         case GT_FIELD_LIST:
+            // These should always be contained. We don't correctly allocate or
+            // generate code for a non-contained GT_FIELD_LIST.
+            noway_assert(!"Non-contained GT_FIELD_LIST");
+            break;
+
+        case GT_LIST:
         case GT_ARGPLACE:
         case GT_NO_OP:
         case GT_START_NONGC:
diff --git a/src/jit/lsraxarch.cpp b/src/jit/lsraxarch.cpp
index b0e95ae31f..5abdb4b6f3 100644
--- a/src/jit/lsraxarch.cpp
+++ b/src/jit/lsraxarch.cpp
@@ -111,8 +111,13 @@ void LinearScan::BuildNode(GenTree* tree)
             BuildStoreLoc(tree->AsLclVarCommon());
             break;
 
-        case GT_LIST:
         case GT_FIELD_LIST:
+            // These should always be contained. We don't correctly allocate or
+            // generate code for a non-contained GT_FIELD_LIST.
+            noway_assert(!"Non-contained GT_FIELD_LIST");
+            break;
+
+        case GT_LIST:
         case GT_ARGPLACE:
         case GT_NO_OP:
         case GT_START_NONGC: