Delete `FixupIfSIMDLocal`. (dotnet/coreclr#20360)
authorSergey Andreenko <seandree@microsoft.com>
Wed, 17 Oct 2018 18:37:20 +0000 (11:37 -0700)
committerGitHub <noreply@github.com>
Wed, 17 Oct 2018 18:37:20 +0000 (11:37 -0700)
* Fix the strange `ifdef ` placement.

* Fix comments/refactoring of `LinearScan::BuildReturn`.

* Delete `FixupIfSIMDLocal`.

Do not change `LCL_FLD(long)` back to `LCL_VAR(simd8)`,

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

src/coreclr/src/jit/lsra.cpp
src/coreclr/src/jit/lsrabuild.cpp
src/coreclr/src/jit/rationalize.cpp
src/coreclr/src/jit/rationalize.h

index 8a613b6..5e4cdcb 100644 (file)
@@ -226,14 +226,14 @@ regMaskTP LinearScan::allRegs(RegisterType rt)
     else if (rt == TYP_DOUBLE)
     {
         return availableDoubleRegs;
-#ifdef FEATURE_SIMD
-        // TODO-Cleanup: Add an RBM_ALLSIMD
     }
+#ifdef FEATURE_SIMD
+    // TODO-Cleanup: Add an RBM_ALLSIMD
     else if (varTypeIsSIMD(rt))
     {
         return availableDoubleRegs;
-#endif // FEATURE_SIMD
     }
+#endif // FEATURE_SIMD
     else
     {
         return availableIntRegs;
index 6dd2c14..8675910 100644 (file)
@@ -2944,12 +2944,11 @@ int LinearScan::BuildSimple(GenTree* tree)
 //    tree - The node of interest
 //
 // Return Value:
-//    None.
+//    The number of sources consumed by this node.
 //
 int LinearScan::BuildReturn(GenTree* tree)
 {
-    int      srcCount = 0;
-    GenTree* op1      = tree->gtGetOp1();
+    GenTree* op1 = tree->gtGetOp1();
 
 #if !defined(_TARGET_64BIT_)
     if (tree->TypeGet() == TYP_LONG)
@@ -2957,9 +2956,9 @@ int LinearScan::BuildReturn(GenTree* tree)
         assert((op1->OperGet() == GT_LONG) && op1->isContained());
         GenTree* loVal = op1->gtGetOp1();
         GenTree* hiVal = op1->gtGetOp2();
-        srcCount       = 2;
         BuildUse(loVal, RBM_LNGRET_LO);
         BuildUse(hiVal, RBM_LNGRET_HI);
+        return 2;
     }
     else
 #endif // !defined(_TARGET_64BIT_)
@@ -2967,8 +2966,6 @@ int LinearScan::BuildReturn(GenTree* tree)
     {
         regMaskTP useCandidates = RBM_NONE;
 
-        srcCount = 1;
-
 #if FEATURE_MULTIREG_RET
         if (varTypeIsStruct(tree))
         {
@@ -2982,12 +2979,13 @@ int LinearScan::BuildReturn(GenTree* tree)
                 noway_assert(op1->IsMultiRegCall());
 
                 ReturnTypeDesc* retTypeDesc = op1->AsCall()->GetReturnTypeDesc();
-                srcCount                    = retTypeDesc->GetReturnRegCount();
+                int             srcCount    = retTypeDesc->GetReturnRegCount();
                 useCandidates               = retTypeDesc->GetABIReturnRegs();
                 for (int i = 0; i < srcCount; i++)
                 {
                     BuildUse(op1, useCandidates, i);
                 }
+                return srcCount;
             }
         }
         else
@@ -3014,11 +3012,12 @@ int LinearScan::BuildReturn(GenTree* tree)
                     break;
             }
             BuildUse(op1, useCandidates);
+            return 1;
         }
     }
-    // No kills or defs
 
-    return srcCount;
+    // No kills or defs.
+    return 0;
 }
 
 //------------------------------------------------------------------------
index 7e72993..ecb072d 100644 (file)
@@ -229,73 +229,6 @@ void Rationalizer::RewriteIntrinsicAsUserCall(GenTree** use, ArrayStack<GenTree*
                       args);
 }
 
-// FixupIfSIMDLocal: Fixup the type of a lclVar tree, as needed, if it is a SIMD type vector.
-//
-// Arguments:
-//    comp      - the Compiler object.
-//    tree      - the GenTreeLclVarCommon tree to be fixed up.
-//
-// Return Value:
-//    None.
-//
-// TODO-1stClassStructs: This is now only here to preserve existing behavior. It is actually not
-// desirable to change the lclFld nodes back to TYP_SIMD (it will cause them to be loaded
-// into a vector register, and then moved to an int register).
-
-void Rationalizer::FixupIfSIMDLocal(GenTreeLclVarCommon* node)
-{
-#ifdef FEATURE_SIMD
-    if (!comp->featureSIMD)
-    {
-        return;
-    }
-
-    LclVarDsc* varDsc = &(comp->lvaTable[node->gtLclNum]);
-
-    // Don't mark byref of SIMD vector as a SIMD type.
-    // Note that struct args though marked as lvIsSIMD=true,
-    // the tree node representing such an arg should not be
-    // marked as a SIMD type, since it is a byref of a SIMD type.
-    if (!varTypeIsSIMD(varDsc))
-    {
-        return;
-    }
-    switch (node->OperGet())
-    {
-        default:
-            // Nothing to do for most tree nodes.
-            break;
-
-        case GT_LCL_FLD:
-            // We may see a lclFld used for pointer-sized structs that have been morphed, in which
-            // case we can change it to GT_LCL_VAR.
-            // However, we may also see a lclFld with FieldSeqStore::NotAField() for structs that can't
-            // be analyzed, e.g. those with overlapping fields such as the IL implementation of Vector<T>.
-            if ((node->AsLclFld()->gtFieldSeq == FieldSeqStore::NotAField()) && (node->AsLclFld()->gtLclOffs == 0) &&
-                (node->gtType == TYP_I_IMPL) && (varDsc->lvExactSize == TARGET_POINTER_SIZE))
-            {
-                node->SetOper(GT_LCL_VAR);
-                node->gtFlags &= ~(GTF_VAR_USEASG);
-            }
-            else
-            {
-                // If we access a field of a SIMD lclVar via GT_LCL_FLD, it cannot have been
-                // independently promoted.
-                assert(comp->lvaGetPromotionType(varDsc) != Compiler::PROMOTION_TYPE_INDEPENDENT);
-                return;
-            }
-            break;
-        case GT_STORE_LCL_FLD:
-            assert(node->gtType == TYP_I_IMPL);
-            node->SetOper(GT_STORE_LCL_VAR);
-            node->gtFlags &= ~(GTF_VAR_USEASG);
-            break;
-    }
-    unsigned simdSize = roundUp(varDsc->lvExactSize, TARGET_POINTER_SIZE);
-    node->gtType      = comp->getSIMDTypeForSize(simdSize);
-#endif // FEATURE_SIMD
-}
-
 #ifdef DEBUG
 
 void Rationalizer::ValidateStatement(GenTree* tree, BasicBlock* block)
@@ -859,12 +792,6 @@ Compiler::fgWalkResult Rationalizer::RewriteNode(GenTree** useEdge, ArrayStack<G
         }
         break;
 
-        case GT_LCL_FLD:
-        case GT_STORE_LCL_FLD:
-            // TODO-1stClassStructs: Eliminate this.
-            FixupIfSIMDLocal(node->AsLclVarCommon());
-            break;
-
         case GT_SIMD:
         {
             noway_assert(comp->featureSIMD);
index 1cfeb2d..7bdddb6 100644 (file)
@@ -38,7 +38,6 @@ private:
 
     // SIMD related
     void RewriteSIMDOperand(LIR::Use& use, bool keepBlk);
-    void FixupIfSIMDLocal(GenTreeLclVarCommon* node);
 
     // Intrinsic related transformations
     void RewriteNodeAsCall(GenTree**             use,