Block the hoisting of TYP_STRUCT rvalues in loop hoisting

dotnet · Apr 8, 2019 · 86d1c3c · 86d1c3c
1 parent e02e6d7
commit 86d1c3c
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 38 deletions.
diff --git a/src/jit/optcse.cpp b/src/jit/optcse.cpp
@@ -2574,34 +2574,11 @@ bool Compiler::optIsCSEcandidate(GenTree* tree)
         return false;
     }
 
-    // For some struct types, we now allow them to be CSE candidates.
-    //
-    if (type == TYP_STRUCT)
+    // If this is a struct type, we can only consider it for CSE-ing if we can get at
+    // its handle, so that we can create a temp.
+    if ((type == TYP_STRUCT) && (gtGetStructHandleIfPresent(tree) == NO_CLASS_HANDLE))
     {
-        // We can only consider it for CSE-ing if we can get at
-        // its handle, so that we can create a temp.
-        //
-        CORINFO_CLASS_HANDLE structHnd = gtGetStructHandleIfPresent(tree);
-        if (structHnd == NO_CLASS_HANDLE)
-        {
-            // Mark this tree with GTF_DONT_CSE so that we don't have to recompute this information
-            tree->gtFlags |= GTF_DONT_CSE;
-            return false;
-        }
-
-        unsigned          size = info.compCompHnd->getClassSize(structHnd);
-        structPassingKind howToPassStruct;
-        var_types         structBaseType = getArgTypeForStruct(structHnd, &howToPassStruct, false, size);
-
-        //
-        // Only allow structs whose storage can be held in a register to be CSE candidates.
-        //
-        if (howToPassStruct != SPK_PrimitiveType)
-        {
-            // Mark this tree with GTF_DONT_CSE so that we don't have to recompute this information
-            tree->gtFlags |= GTF_DONT_CSE;
-            return false;
-        }
+        return false;
     }
 
 #ifdef _TARGET_X86_

diff --git a/src/jit/optimizer.cpp b/src/jit/optimizer.cpp
@@ -6885,26 +6885,35 @@ bool Compiler::optHoistLoopExprsForTree(GenTree*          tree,
         // Tree must be a suitable CSE candidate for us to be able to hoist it.
         treeIsHoistable &= optIsCSEcandidate(tree);
 
-        // If it's a call, it must be a helper call, and be pure.
-        // Further, if it may run a cctor, it must be labeled as "Hoistable"
-        // (meaning it won't run a cctor because the class is not precise-init).
-        if (treeIsHoistable && tree->OperGet() == GT_CALL)
+        if (treeIsHoistable)
         {
-            GenTreeCall* call = tree->AsCall();
-            if (call->gtCallType != CT_HELPER)
+            // We cannot hoist an r-value of TYP_STRUCT, as it is illegal to do so
+            if (tree->TypeGet() == TYP_STRUCT)
             {
                 treeIsHoistable = false;
             }
-            else
+
+            // If it's a call, it must be a helper call, and be pure.
+            // Further, if it may run a cctor, it must be labeled as "Hoistable"
+            // (meaning it won't run a cctor because the class is not precise-init).
+            if (tree->OperGet() == GT_CALL)
             {
-                CorInfoHelpFunc helpFunc = eeGetHelperNum(call->gtCallMethHnd);
-                if (!s_helperCallProperties.IsPure(helpFunc))
+                GenTreeCall* call = tree->AsCall();
+                if (call->gtCallType != CT_HELPER)
                 {
                     treeIsHoistable = false;
                 }
-                else if (s_helperCallProperties.MayRunCctor(helpFunc) && (call->gtFlags & GTF_CALL_HOISTABLE) == 0)
+                else
                 {
-                    treeIsHoistable = false;
+                    CorInfoHelpFunc helpFunc = eeGetHelperNum(call->gtCallMethHnd);
+                    if (!s_helperCallProperties.IsPure(helpFunc))
+                    {
+                        treeIsHoistable = false;
+                    }
+                    else if (s_helperCallProperties.MayRunCctor(helpFunc) && (call->gtFlags & GTF_CALL_HOISTABLE) == 0)
+                    {
+                        treeIsHoistable = false;
+                    }
                 }
             }
         }