Forbid block morph in CSE. (#33845)
authorSergey Andreenko <seandree@microsoft.com>
Fri, 20 Mar 2020 19:40:16 +0000 (12:40 -0700)
committerGitHub <noreply@github.com>
Fri, 20 Mar 2020 19:40:16 +0000 (12:40 -0700)
* Don't call `fgMorphCopyBlock` during CSE.

It could do tranformations that replace trees, some of them could be CSE defs or uses, it would lead to asserts when we try to find them.

* Dump additional information about struct localVariables.

src/coreclr/src/jit/compiler.h
src/coreclr/src/jit/gentree.cpp
src/coreclr/src/jit/lclvars.cpp
src/coreclr/src/jit/morph.cpp

index 9afa3fe..64b719f 100644 (file)
@@ -2890,7 +2890,9 @@ public:
     void gtGetLclVarNameInfo(unsigned lclNum, const char** ilKindOut, const char** ilNameOut, unsigned* ilNumOut);
     int gtGetLclVarName(unsigned lclNum, char* buf, unsigned buf_remaining);
     char* gtGetLclVarName(unsigned lclNum);
-    void gtDispLclVar(unsigned varNum, bool padForBiggestDisp = true);
+    void gtDispLclVar(unsigned lclNum, bool padForBiggestDisp = true);
+    void gtDispLclVarStructType(unsigned lclNum);
+    void gtDispClassLayout(ClassLayout* layout, var_types type);
     void gtDispStmt(Statement* stmt, const char* msg = nullptr);
     void gtDispBlockStmts(BasicBlock* block);
     void gtGetArgMsg(GenTreeCall* call, GenTree* arg, unsigned argNum, int listCount, char* bufp, unsigned bufLength);
index 53efb3e..f0416bb 100644 (file)
@@ -10035,18 +10035,7 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, __in __in_z _
 
                 if (layout != nullptr)
                 {
-                    if (layout->IsBlockLayout())
-                    {
-                        printf("<%u>", layout->GetSize());
-                    }
-                    else if (varTypeIsSIMD(tree->TypeGet()))
-                    {
-                        printf("<%s>", layout->GetClassName());
-                    }
-                    else
-                    {
-                        printf("<%s, %u>", layout->GetClassName(), layout->GetSize());
-                    }
+                    gtDispClassLayout(layout, tree->TypeGet());
                 }
             }
 
@@ -10404,6 +10393,69 @@ void Compiler::gtDispLclVar(unsigned lclNum, bool padForBiggestDisp)
     }
 }
 
+//------------------------------------------------------------------------
+// gtDispLclVarStructType: Print size and type information about a struct or lclBlk local variable.
+//
+// Arguments:
+//   lclNum - The local var id.
+//
+void Compiler::gtDispLclVarStructType(unsigned lclNum)
+{
+    LclVarDsc* varDsc = lvaGetDesc(lclNum);
+    var_types  type   = varDsc->TypeGet();
+    if (type == TYP_STRUCT)
+    {
+        ClassLayout* layout = varDsc->GetLayout();
+        assert(layout != nullptr);
+        gtDispClassLayout(layout, type);
+    }
+    else if (type == TYP_LCLBLK)
+    {
+#if FEATURE_FIXED_OUT_ARGS
+        assert(lclNum == lvaOutgoingArgSpaceVar);
+        // Since lvaOutgoingArgSpaceSize is a PhasedVar we can't read it for Dumping until
+        // after we set it to something.
+        if (lvaOutgoingArgSpaceSize.HasFinalValue())
+        {
+            // A PhasedVar<T> can't be directly used as an arg to a variadic function
+            unsigned value = lvaOutgoingArgSpaceSize;
+            printf("<%u> ", value);
+        }
+        else
+        {
+            printf("<na> "); // The value hasn't yet been determined
+        }
+#else
+        assert(!"Unknown size");
+        NO_WAY("Target doesn't support TYP_LCLBLK");
+#endif // FEATURE_FIXED_OUT_ARGS
+    }
+}
+
+//------------------------------------------------------------------------
+// gtDispClassLayout: Print size and type information about a layout.
+//
+// Arguments:
+//   layout - the layout;
+//   type   - variable type, used to avoid printing size for SIMD nodes.
+//
+void Compiler::gtDispClassLayout(ClassLayout* layout, var_types type)
+{
+    assert(layout != nullptr);
+    if (layout->IsBlockLayout())
+    {
+        printf("<%u>", layout->GetSize());
+    }
+    else if (varTypeIsSIMD(type))
+    {
+        printf("<%s>", layout->GetClassName());
+    }
+    else
+    {
+        printf("<%s, %u>", layout->GetClassName(), layout->GetSize());
+    }
+}
+
 /*****************************************************************************/
 void Compiler::gtDispConst(GenTree* tree)
 {
index 76e17f0..8ee7c10 100644 (file)
@@ -6847,7 +6847,7 @@ void Compiler::lvaDumpFrameLocation(unsigned lclNum)
 
 void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t refCntWtdWidth)
 {
-    LclVarDsc* varDsc = lvaTable + lclNum;
+    LclVarDsc* varDsc = lvaGetDesc(lclNum);
     var_types  type   = varDsc->TypeGet();
 
     if (curState == INITIAL_FRAME_LAYOUT)
@@ -6856,30 +6856,7 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r
         gtDispLclVar(lclNum);
 
         printf(" %7s ", varTypeName(type));
-        if (genTypeSize(type) == 0)
-        {
-#if FEATURE_FIXED_OUT_ARGS
-            if (lclNum == lvaOutgoingArgSpaceVar)
-            {
-                // Since lvaOutgoingArgSpaceSize is a PhasedVar we can't read it for Dumping until
-                // after we set it to something.
-                if (lvaOutgoingArgSpaceSize.HasFinalValue())
-                {
-                    // A PhasedVar<T> can't be directly used as an arg to a variadic function
-                    unsigned value = lvaOutgoingArgSpaceSize;
-                    printf("(%2d) ", value);
-                }
-                else
-                {
-                    printf("(na) "); // The value hasn't yet been determined
-                }
-            }
-            else
-#endif // FEATURE_FIXED_OUT_ARGS
-            {
-                printf("(%2d) ", lvaLclSize(lclNum));
-            }
-        }
+        gtDispLclVarStructType(lclNum);
     }
     else
     {
index 4f3a141..5dce49a 100644 (file)
@@ -13757,6 +13757,13 @@ GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree)
     switch (oper)
     {
         case GT_ASG:
+            // Make sure we're allowed to do this.
+            if (optValnumCSE_phase)
+            {
+                // It is not safe to reorder/delete CSE's
+                break;
+            }
+
             if (varTypeIsStruct(typ) && !tree->IsPhiDefn())
             {
                 if (tree->OperIsCopyBlkOp())
@@ -13774,14 +13781,6 @@ GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree)
                 break;
             }
 
-            /* Make sure we're allowed to do this */
-
-            if (optValnumCSE_phase)
-            {
-                // It is not safe to reorder/delete CSE's
-                break;
-            }
-
             if (op2->gtFlags & GTF_ASG)
             {
                 break;