Refactor isContained().

This changes isContained to terminate early if it is not possible for the node in question
to ever be contained. There are many nodes that are statically non-containable: any node
that is not a value may not be contained along with a small number of value nodes. To avoid
the need for a `switch` to identify the latter, their node kinds have been marked with a
new flag, `GTK_NOCONTAIN`. The checks for identifying non-containable nodes are encapsulated
withing a new function, `canBeContained`.

Commit migrated from https://github.com/dotnet/coreclr/commit/350020d9b6a2bda08cf2e416d959c1801a717734

diff --git a/src/coreclr/src/jit/gentree.cpp b/src/coreclr/src/jit/gentree.cpp
index 7af500f..57c5908 100644 (file)
--- a/src/coreclr/src/jit/gentree.cpp
+++ b/src/coreclr/src/jit/gentree.cpp
@@ -15787,6 +15787,20 @@ unsigned GenTree::IsLclVarUpdateTree(GenTree** pOtherTree, genTreeOps* pOper)
 }
 
 //------------------------------------------------------------------------
+// canBeContained: check whether this tree node may be a subcomponent of its parent for purposes
+//                 of code generation.
+//
+// Return value: returns true if it is possible to contain this node and false otherwise.
+bool GenTree::canBeContained() const
+{
+    assert(IsLIR());
+
+    // It is not possible for nodes that do not produce values or that are not containable values
+    // to be contained.
+    return (OperKind() & (GTK_NOVALUE | GTK_NOCONTAIN)) == 0;
+}
+
+//------------------------------------------------------------------------
 // isContained: check whether this tree node is a subcomponent of its parent for codegen purposes
 //
 // Return Value:
@@ -15801,14 +15815,16 @@ unsigned GenTree::IsLclVarUpdateTree(GenTree** pOtherTree, genTreeOps* pOper)
 //
 bool GenTree::isContained() const
 {
-    if (gtHasReg())
+    assert(IsLIR());
+
+    if (!canBeContained() || gtHasReg())
     {
         return false;
     }
 
     // these actually produce a register (the flags reg, we just don't model it)
     // and are a separate instruction from the branch that consumes the result
-    if (OperKind() & GTK_RELOP)
+    if ((OperKind() & GTK_RELOP) != 0)
     {
         return false;
     }
@@ -15819,75 +15835,25 @@ bool GenTree::isContained() const
         return false;
     }
 
-    switch (OperGet())
-    {
-        case GT_STOREIND:
-        case GT_JTRUE:
-        case GT_JCC:
-        case GT_RETURN:
-        case GT_RETFILT:
-        case GT_STORE_LCL_FLD:
-        case GT_STORE_LCL_VAR:
-        case GT_ARR_BOUNDS_CHECK:
-        case GT_LOCKADD:
-        case GT_NOP:
-        case GT_NO_OP:
-        case GT_START_NONGC:
-        case GT_PROF_HOOK:
-        case GT_RETURNTRAP:
-        case GT_COMMA:
-        case GT_PINVOKE_PROLOG:
-        case GT_PHYSREGDST:
-        case GT_PUTARG_STK:
-        case GT_MEMORYBARRIER:
-        case GT_STORE_BLK:
-        case GT_STORE_OBJ:
-        case GT_STORE_DYN_BLK:
-        case GT_SWITCH:
-#ifndef LEGACY_BACKEND
-        case GT_JMPTABLE:
-#endif
-        case GT_SWITCH_TABLE:
-        case GT_SWAP:
-        case GT_LCLHEAP:
-        case GT_CKFINITE:
-        case GT_JMP:
-        case GT_IL_OFFSET:
-#ifdef FEATURE_SIMD
-        case GT_SIMD_CHK:
-#endif // FEATURE_SIMD
-
-#if !FEATURE_EH_FUNCLETS
-        case GT_END_LFIN:
-#endif
-            return false;
-
 #if !defined(LEGACY_BACKEND) && !defined(_TARGET_64BIT_)
-        case GT_LONG:
-            // GT_LONG nodes are normally contained. The only exception is when the result
-            // of a TYP_LONG operation is not used and this can only happen if the GT_LONG
-            // is the last node in the statement (in linear order).
-            return gtNext != nullptr;
+    if (OperGet() == GT_LONG)
+    {
+        // GT_LONG nodes are normally contained. The only exception is when the result
+        // of a TYP_LONG operation is not used and this can only happen if the GT_LONG
+        // is the last node in the statement (in linear order).
+        return gtNext != nullptr;
+    }
 #endif
 
-        case GT_CALL:
-            // Note: if you hit this assert you are probably calling isContained()
-            // before the LSRA phase has allocated physical register to the tree nodes
-            //
-            assert(gtType == TYP_VOID);
-            return false;
-
-        default:
-            // if it's contained it better have a parent
-            assert(gtNext || OperIsLocal());
-            return true;
-    }
+    // if it's contained it better have a user
+    assert((gtNext != nullptr) || OperIsLocal());
+    return true;
 }
 
 // return true if node is contained and an indir
 bool GenTree::isContainedIndir() const
 {
-    return isContained() && isIndir();
+    return isIndir() && isContained();
 }
 
 bool GenTree::isIndirAddrMode()

diff --git a/src/coreclr/src/jit/gentree.h b/src/coreclr/src/jit/gentree.h
index eaca7a0..cd1034d 100644 (file)
--- a/src/coreclr/src/jit/gentree.h
+++ b/src/coreclr/src/jit/gentree.h
@@ -120,6 +120,8 @@ enum genTreeKinds
     GTK_NOVALUE = 0x0400, // node does not produce a value
     GTK_NOTLIR  = 0x0800, // node is not allowed in LIR
 
+    GTK_NOCONTAIN = 0x1000, // this node is a value, but may not be contained
+
     /* Define composite value(s) */
 
     GTK_SMPOP = (GTK_UNOP | GTK_BINOP | GTK_RELOP | GTK_LOGOP)
@@ -555,6 +557,8 @@ public:
 
     __declspec(property(get = GetRegNum, put = SetRegNum)) regNumber gtRegNum;
 
+    bool canBeContained() const;
+
     // for codegen purposes, is this node a subnode of its parent
     bool isContained() const;
 

diff --git a/src/coreclr/src/jit/gtlist.h b/src/coreclr/src/jit/gtlist.h
index 2d9255b..826eaf1 100644 (file)
--- a/src/coreclr/src/jit/gtlist.h
+++ b/src/coreclr/src/jit/gtlist.h
@@ -46,7 +46,7 @@ GTNODE(CNS_STR          , "sconst"       ,GenTreeStrCon      ,0,GTK_LEAF|GTK_CON
 //-----------------------------------------------------------------------------
 
 GTNODE(NOT              , "~"            ,GenTreeOp          ,0,GTK_UNOP)
-GTNODE(NOP              , "nop"          ,GenTree            ,0,GTK_UNOP)
+GTNODE(NOP              , "nop"          ,GenTree            ,0,GTK_UNOP|GTK_NOCONTAIN)
 GTNODE(NEG              , "unary -"      ,GenTreeOp          ,0,GTK_UNOP)
 GTNODE(COPY             , "copy"         ,GenTreeCopyOrReload,0,GTK_UNOP)               // Copies a variable from its current location to a register that satisfies
                                                                                         // code generation constraints.  The child is the actual lclVar node.
@@ -65,8 +65,8 @@ GTNODE(CMPXCHG          , "cmpxchg"      ,GenTreeCmpXchg     ,0,GTK_SPECIAL)
 GTNODE(MEMORYBARRIER    , "memoryBarrier",GenTree            ,0,GTK_LEAF|GTK_NOVALUE)
 
 GTNODE(CAST             , "cast"         ,GenTreeCast        ,0,GTK_UNOP|GTK_EXOP)      // conversion to another type
-GTNODE(CKFINITE         , "ckfinite"     ,GenTreeOp          ,0,GTK_UNOP)               // Check for NaN
-GTNODE(LCLHEAP          , "lclHeap"      ,GenTreeOp          ,0,GTK_UNOP)               // alloca()
+GTNODE(CKFINITE         , "ckfinite"     ,GenTreeOp          ,0,GTK_UNOP|GTK_NOCONTAIN) // Check for NaN
+GTNODE(LCLHEAP          , "lclHeap"      ,GenTreeOp          ,0,GTK_UNOP|GTK_NOCONTAIN) // alloca()
 GTNODE(JMP              , "jump"         ,GenTreeVal         ,0,GTK_LEAF|GTK_NOVALUE)   // Jump to another function
 
 GTNODE(ADDR             , "addr"         ,GenTreeOp          ,0,GTK_UNOP)               // address of
@@ -226,7 +226,7 @@ GTNODE(FIELD            , "field"        ,GenTreeField       ,0,GTK_SPECIAL)
 GTNODE(ARR_ELEM         , "arrMD&"       ,GenTreeArrElem     ,0,GTK_SPECIAL)            // Multi-dimensional array-element address
 GTNODE(ARR_INDEX        , "arrMDIdx"     ,GenTreeArrIndex    ,0,GTK_BINOP|GTK_EXOP)     // Effective, bounds-checked index for one dimension of a multi-dimensional array element
 GTNODE(ARR_OFFSET       , "arrMDOffs"    ,GenTreeArrOffs     ,0,GTK_SPECIAL)            // Flattened offset of multi-dimensional array element
-GTNODE(CALL             , "call()"       ,GenTreeCall        ,0,GTK_SPECIAL)
+GTNODE(CALL             , "call()"       ,GenTreeCall        ,0,GTK_SPECIAL|GTK_NOCONTAIN)
 
 //-----------------------------------------------------------------------------
 //  Statement operator nodes:
@@ -261,7 +261,7 @@ GTNODE(PHI_ARG          , "phiArg"       ,GenTreePhiArg      ,0,GTK_LEAF|GTK_LOC
 //-----------------------------------------------------------------------------
 
 #ifndef LEGACY_BACKEND
-GTNODE(JMPTABLE         , "jumpTable"    ,GenTreeJumpTable   ,0, GTK_LEAF)              // Generates the jump table for switches
+GTNODE(JMPTABLE         , "jumpTable"    ,GenTreeJumpTable   ,0, GTK_LEAF|GTK_NOCONTAIN) // Generates the jump table for switches
 #endif
 GTNODE(SWITCH_TABLE     , "tableSwitch"  ,GenTreeOp          ,0, GTK_BINOP|GTK_NOVALUE) // Jump Table based switch construct
 

diff --git a/src/coreclr/src/jit/lower.cpp b/src/coreclr/src/jit/lower.cpp
index 2113e08..33931c6 100644 (file)
--- a/src/coreclr/src/jit/lower.cpp
+++ b/src/coreclr/src/jit/lower.cpp
@@ -42,9 +42,12 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 void Lowering::MakeSrcContained(GenTreePtr parentNode, GenTreePtr childNode)
 {
     assert(!parentNode->OperIsLeaf());
+    assert(childNode->canBeContained());
+
     int srcCount = childNode->gtLsraInfo.srcCount;
     assert(srcCount >= 0);
     m_lsra->clearOperandCounts(childNode);
+
     assert(parentNode->gtLsraInfo.srcCount > 0);
     parentNode->gtLsraInfo.srcCount += srcCount - 1;
 }