Revert "JIT: revise how we track if nodes have been morphed (#110787)"

This reverts commit 4822cc0.

Author: MichalStrehovsky

src/coreclr/jit/compiler.h

@@ -5438,10 +5438,6 @@ class Compiler
     void fgMorphBlock(BasicBlock* block, MorphUnreachableInfo* unreachableInfo = nullptr);
     void fgMorphStmts(BasicBlock* block);
 
-#ifdef DEBUG
-    void fgPostGlobalMorphChecks();
-#endif
-
     void fgMergeBlockReturn(BasicBlock* block);
 
     bool fgMorphBlockStmt(BasicBlock* block, Statement* stmt DEBUGARG(const char* msg), bool invalidateDFSTreeOnFGChange = true);
@@ -6789,7 +6785,7 @@ class Compiler
     void fgKillDependentAssertionsSingle(unsigned lclNum DEBUGARG(GenTree* tree));
     void fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree));
     void fgMorphTreeDone(GenTree* tree);
-    void fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone DEBUGARG(int morphNum = 0));
+    void fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone, bool isMorphedTree DEBUGARG(int morphNum = 0));
 
     Statement* fgMorphStmt;
     unsigned   fgBigOffsetMorphingTemps[TYP_COUNT];

src/coreclr/jit/compiler.hpp

@@ -1354,7 +1354,6 @@ inline GenTree::GenTree(genTreeOps oper, var_types type DEBUGARG(bool largeNode)
     gtLIRFlags = 0;
 #ifdef DEBUG
     gtDebugFlags = GTF_DEBUG_NONE;
-    gtMorphCount = 0;
 #endif // DEBUG
     gtCSEnum = NO_CSE;
     ClearAssertion();

src/coreclr/jit/gentree.cpp

@@ -4903,6 +4903,7 @@ static void SetIndirectStoreEvalOrder(Compiler* comp, GenTreeIndir* store, bool*
  *      1. GetCostEx() to the execution complexity estimate
  *      2. GetCostSz() to the code size estimate
  *      3. Sometimes sets GTF_ADDRMODE_NO_CSE on nodes in the tree.
+ *      4. DEBUG-only: clears GTF_DEBUG_NODE_MORPHED.
  */
 
 #ifdef _PREFAST_
@@ -14387,7 +14388,10 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
     // Helper function that creates a new IntCon node and morphs it, if required
     auto NewMorphedIntConNode = [&](int value) -> GenTreeIntCon* {
         GenTreeIntCon* icon = gtNewIconNode(value);
-        icon->SetMorphed(this);
+        if (fgGlobalMorph)
+        {
+            INDEBUG(icon->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+        }
         return icon;
     };
 
@@ -14398,14 +14402,18 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
         assert(varTypeIsUnsigned(castToType));
 
         GenTreeCast* cast = gtNewCastNode(TYP_INT, op1, false, castToType);
-        cast->SetMorphed(this);
-        fgMorphTreeDone(cast);
+        if (fgGlobalMorph)
+        {
+            fgMorphTreeDone(cast);
+        }
 
         if (type == TYP_LONG)
         {
             cast = gtNewCastNode(TYP_LONG, cast, true, TYP_LONG);
-            cast->SetMorphed(this);
-            fgMorphTreeDone(cast);
+            if (fgGlobalMorph)
+            {
+                fgMorphTreeDone(cast);
+            }
         }
 
         return cast;
@@ -14672,7 +14680,14 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
     DISPTREE(tree);
     JITDUMP("Transformed into:\n");
     DISPTREE(op);
-    op->SetMorphed(this);
+
+    if (fgGlobalMorph)
+    {
+        // We can sometimes produce a comma over the constant if the original op
+        // had a side effect, so just ensure we set the flag (which will be already
+        // set for the operands otherwise).
+        INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+    }
     return op;
 }
 
@@ -14727,9 +14742,10 @@ GenTree* Compiler::gtFoldExprSpecialFloating(GenTree* tree)
     // Helper function that creates a new IntCon node and morphs it, if required
     auto NewMorphedIntConNode = [&](int value) -> GenTreeIntCon* {
         GenTreeIntCon* icon = gtNewIconNode(value);
-
-        icon->SetMorphed(this);
-
+        if (fgGlobalMorph)
+        {
+            INDEBUG(icon->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+        }
         return icon;
     };
 
@@ -14891,8 +14907,14 @@ GenTree* Compiler::gtFoldExprSpecialFloating(GenTree* tree)
     DISPTREE(tree);
     JITDUMP("Transformed into:\n");
     DISPTREE(op);
-    op->SetMorphed(this);
 
+    if (fgGlobalMorph)
+    {
+        // We can sometimes produce a comma over the constant if the original op
+        // had a side effect, so just ensure we set the flag (which will be already
+        // set for the operands otherwise).
+        INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+    }
     return op;
 }
 
@@ -15413,11 +15435,6 @@ GenTree* Compiler::gtOptimizeEnumHasFlag(GenTree* thisOp, GenTree* flagOp)
         Statement*     thisStoreStmt = thisOp->AsBox()->gtCopyStmtWhenInlinedBoxValue;
         thisStoreStmt->SetRootNode(thisStore);
         thisValOpt = gtNewLclvNode(thisTmp, type);
-
-        // If this is invoked during global morph we are adding code to a remote tree
-        // Despite this being a store, we can't meaningfully add assertions
-        //
-        thisStore->SetMorphed(this);
     }
 
     if (flagVal->IsIntegralConst())
@@ -15435,11 +15452,6 @@ GenTree* Compiler::gtOptimizeEnumHasFlag(GenTree* thisOp, GenTree* flagOp)
         flagStoreStmt->SetRootNode(flagStore);
         flagValOpt     = gtNewLclvNode(flagTmp, type);
         flagValOptCopy = gtNewLclvNode(flagTmp, type);
-
-        // If this is invoked during global morph we are adding code to a remote tree
-        // Despite this being a store, we can't meaningfully add assertions
-        //
-        flagStore->SetMorphed(this);
     }
 
     // Turn the call into (thisValTmp & flagTmp) == flagTmp.
@@ -17424,7 +17436,16 @@ void Compiler::gtExtractSideEffList(GenTree*     expr,
             }
 
             GenTree* comma = m_compiler->gtNewOperNode(GT_COMMA, TYP_VOID, m_result, node);
-            comma->SetMorphed(m_compiler);
+
+#ifdef DEBUG
+            if (m_compiler->fgGlobalMorph)
+            {
+                // Either both should be morphed or neither should be.
+                assert((m_result->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) ==
+                       (node->gtDebugFlags & GTF_DEBUG_NODE_MORPHED));
+                comma->gtDebugFlags |= node->gtDebugFlags & GTF_DEBUG_NODE_MORPHED;
+            }
+#endif
 
             // Both should have valuenumbers defined for both or for neither
             // one (unless we are remorphing, in which case a prior transform
@@ -30741,7 +30762,12 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
         }
 
         vecCon->gtSimdVal = simdVal;
-        vecCon->SetMorphed(this);
+
+        if (fgGlobalMorph)
+        {
+            INDEBUG(vecCon->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+        }
+
         fgUpdateConstTreeValueNumber(vecCon);
         return vecCon;
     }
@@ -30803,7 +30829,11 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
 #endif // !TARGET_XARCH && !TARGET_ARM64
 
                     DEBUG_DESTROY_NODE(op, tree);
-                    vectorNode->SetMorphed(this);
+
+                    if (fgGlobalMorph)
+                    {
+                        INDEBUG(vectorNode->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+                    }
                     return vectorNode;
                 }
             }
@@ -31988,10 +32018,17 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
 
     if (resultNode != tree)
     {
-        resultNode->SetMorphed(this);
-        if (resultNode->OperIs(GT_COMMA))
+        if (fgGlobalMorph)
         {
-            resultNode->AsOp()->gtGetOp2()->SetMorphed(this);
+            // We can sometimes produce a comma over the constant if the original op
+            // had a side effect or even a new constant node, so just ensure we set
+            // the flag (which will be already set for the operands otherwise).
+            INDEBUG(resultNode->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+
+            if (resultNode->OperIs(GT_COMMA))
+            {
+                INDEBUG(resultNode->AsOp()->gtGetOp2()->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
+            }
         }
 
         if (resultNode->OperIsConst())
@@ -32118,65 +32155,3 @@ bool Compiler::gtCanSkipCovariantStoreCheck(GenTree* value, GenTree* array)
 
     return false;
 }
-
-#if defined(DEBUG)
-//------------------------------------------------------------------------
-// SetMorphed: mark a node as having been morphed
-//
-// Arguments:
-//   compiler - compiler instance
-//   doChildren - recursive mark child nodes
-//
-// Notes:
-//   Does nothing outside of global morph.
-//
-//   Useful for morph post-order expansions / optimizations.
-//
-//   Use care when invoking this on an assignment (or when doChildren is true,
-//   on trees containing assignments) as those usually will also require
-//   local assertion updates.
-//
-void GenTree::SetMorphed(Compiler* compiler, bool doChildren /* = false */)
-{
-    if (!compiler->fgGlobalMorph)
-    {
-        return;
-    }
-
-    struct Visitor : GenTreeVisitor<Visitor>
-    {
-        enum
-        {
-            DoPostOrder = true,
-        };
-
-        Visitor(Compiler* comp)
-            : GenTreeVisitor(comp)
-        {
-        }
-
-        fgWalkResult PostOrderVisit(GenTree** use, GenTree* user)
-        {
-            GenTree* const node = *use;
-            if (!node->WasMorphed())
-            {
-                node->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
-                node->gtMorphCount++;
-            }
-            return Compiler::WALK_CONTINUE;
-        }
-    };
-
-    if (doChildren)
-    {
-        Visitor  v(compiler);
-        GenTree* node = this;
-        v.WalkTree(&node, nullptr);
-    }
-    else if (!WasMorphed())
-    {
-        gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
-        gtMorphCount++;
-    }
-}
-#endif

src/coreclr/jit/gentree.h

@@ -602,21 +602,21 @@ inline GenTreeFlags& operator ^=(GenTreeFlags& a, GenTreeFlags b)
 //------------------------------------------------------------------------
 // GenTreeDebugFlags: a bitmask of debug-only flags for GenTree stored in gtDebugFlags
 //
-enum GenTreeDebugFlags : unsigned short
+enum GenTreeDebugFlags : unsigned int
 {
-    GTF_DEBUG_NONE              = 0x0000, // No debug flags.
+    GTF_DEBUG_NONE              = 0x00000000, // No debug flags.
 
-    GTF_DEBUG_NODE_MORPHED      = 0x0001, // the node has been morphed (in the global morphing phase)
-    GTF_DEBUG_NODE_SMALL        = 0x0002,
-    GTF_DEBUG_NODE_LARGE        = 0x0004,
-    GTF_DEBUG_NODE_CG_PRODUCED  = 0x0008, // genProduceReg has been called on this node
-    GTF_DEBUG_NODE_CG_CONSUMED  = 0x0010, // genConsumeReg has been called on this node
-    GTF_DEBUG_NODE_LSRA_ADDED   = 0x0020, // This node was added by LSRA
+    GTF_DEBUG_NODE_MORPHED      = 0x00000001, // the node has been morphed (in the global morphing phase)
+    GTF_DEBUG_NODE_SMALL        = 0x00000002,
+    GTF_DEBUG_NODE_LARGE        = 0x00000004,
+    GTF_DEBUG_NODE_CG_PRODUCED  = 0x00000008, // genProduceReg has been called on this node
+    GTF_DEBUG_NODE_CG_CONSUMED  = 0x00000010, // genConsumeReg has been called on this node
+    GTF_DEBUG_NODE_LSRA_ADDED   = 0x00000020, // This node was added by LSRA
 
-    GTF_DEBUG_NODE_MASK         = 0x003F, // These flags are all node (rather than operation) properties.
+    GTF_DEBUG_NODE_MASK         = 0x0000003F, // These flags are all node (rather than operation) properties.
 
-    GTF_DEBUG_VAR_CSE_REF       = 0x8000, // GT_LCL_VAR -- This is a CSE LCL_VAR node
-    GTF_DEBUG_CAST_DONT_FOLD    = 0x4000, // GT_CAST    -- Try to prevent this cast from being folded
+    GTF_DEBUG_VAR_CSE_REF       = 0x00800000, // GT_LCL_VAR -- This is a CSE LCL_VAR node
+    GTF_DEBUG_CAST_DONT_FOLD    = 0x00400000, // GT_CAST    -- Try to prevent this cast from being folded
 };
 
 inline constexpr GenTreeDebugFlags operator ~(GenTreeDebugFlags a)
@@ -972,26 +972,7 @@ struct GenTree
 
 #if defined(DEBUG)
     GenTreeDebugFlags gtDebugFlags;
-    unsigned short    gtMorphCount;
-    void              SetMorphed(Compiler* compiler, bool doChilren = false);
-
-    bool WasMorphed() const
-    {
-        return (gtDebugFlags & GTF_DEBUG_NODE_MORPHED) != 0;
-    }
-
-    void ClearMorphed()
-    {
-        gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED;
-    }
-#else
-    void SetMorphed(Compiler* compiler, bool doChildren = false)
-    {
-    }
-    void ClearMorphed()
-    {
-    }
-#endif
+#endif // defined(DEBUG)
 
     ValueNumPair gtVNPair;