Delete `gtNewBlkOpNode` and `lvSimdBaseJitType` (#85848)

* Delete "gtNewBlkOpNode"

Note: neither "initobj" nor "cpobj" can be preceded by "volatile.".

Delete the self-assignment handling

Block morphing will fold it to a NOP anyway.

I do not see the referenced liveness bug still being present.
It seems to have been a remnant of COPYBLK/COPYOBJ days.

Move the SIMD logic to gtNewAssignNode

Delete "gtNewBlkOpNode"

* Delete lvSimdBaseJitType

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index 91d4350..53cf934 100644 (file)
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -626,21 +626,8 @@ public:
 
 #ifdef FEATURE_SIMD
     unsigned char lvUsedInSIMDIntrinsic : 1; // This tells lclvar is used for simd intrinsic
-    unsigned char lvSimdBaseJitType : 5;     // Note: this only packs because CorInfoType has less than 32 entries
+#endif                                       // FEATURE_SIMD
 
-    CorInfoType GetSimdBaseJitType() const
-    {
-        return (CorInfoType)lvSimdBaseJitType;
-    }
-
-    void SetSimdBaseJitType(CorInfoType simdBaseJitType)
-    {
-        assert(simdBaseJitType < (1 << 5));
-        lvSimdBaseJitType = (unsigned char)simdBaseJitType;
-    }
-
-    var_types GetSimdBaseType() const;
-#endif                             // FEATURE_SIMD
     unsigned char lvRegStruct : 1; // This is a reg-sized non-field-addressed struct.
 
     unsigned char lvClassIsExact : 1; // lvClassHandle is the exact type
@@ -953,11 +940,6 @@ public:
         return lvUsedInSIMDIntrinsic;
     }
 #else
-    // If feature_simd not enabled, return false
-    bool lvIsSIMDType() const
-    {
-        return false;
-    }
     bool lvIsUsedInSIMDIntrinsic() const
     {
         return false;
@@ -2524,8 +2506,6 @@ public:
 
     GenTreeLclFld* gtNewStoreLclFldNode(unsigned lclNum, var_types type, unsigned offset, GenTree* data);
 
-    GenTree* gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, bool isVolatile = false);
-
     GenTree* gtNewPutArgReg(var_types type, GenTree* arg, regNumber argReg);
 
     GenTree* gtNewBitCastNode(var_types type, GenTree* arg);

diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp
index 856432f..fda2969 100644 (file)
--- a/src/coreclr/jit/fginline.cpp
+++ b/src/coreclr/jit/fginline.cpp
@@ -384,7 +384,7 @@ private:
         }
 
         GenTree* dst = m_compiler->gtNewLclvNode(lclNum, varDsc->TypeGet());
-        GenTree* asg = m_compiler->gtNewBlkOpNode(dst, src);
+        GenTree* asg = m_compiler->gtNewAssignNode(dst, src);
 
         // If inlinee was comma, new inlinee is (, , , lcl = inlinee).
         if (inlinee->OperIs(GT_COMMA))

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
index 676f6fa..ead839c 100644 (file)
--- a/src/coreclr/jit/gentree.cpp
+++ b/src/coreclr/jit/gentree.cpp
@@ -8270,17 +8270,23 @@ GenTreeOp* Compiler::gtNewAssignNode(GenTree* dst, GenTree* src)
     }
     dst->gtFlags |= GTF_DONT_CSE;
 
-#if defined(FEATURE_SIMD) && !defined(TARGET_X86)
-    // TODO-Cleanup: enable on Windows x86.
+#if defined(FEATURE_SIMD)
+#if !defined(TARGET_X86)
     if (varTypeIsSIMD(dst))
     {
         // We want to track SIMD assignments as being intrinsics since they
         // are functionally SIMD `mov` instructions and are more efficient
         // when we don't promote, particularly when it occurs due to inlining
-
         SetOpLclRelatedToSIMDIntrinsic(dst);
         SetOpLclRelatedToSIMDIntrinsic(src);
     }
+#else  // TARGET_X86
+    // TODO-Cleanup: merge with the all-arch logic.
+    if (varTypeIsSIMD(src) && src->OperIs(GT_HWINTRINSIC, GT_CNS_VEC))
+    {
+        SetOpLclRelatedToSIMDIntrinsic(dst);
+    }
+#endif // TARGET_X86
 #endif // FEATURE_SIMD
 
     /* Create the assignment node */
@@ -8508,86 +8514,6 @@ void GenTreeOp::CheckDivideByConstOptimized(Compiler* comp)
 }
 
 //------------------------------------------------------------------------
-// gtNewBlkOpNode: Creates a GenTree for a block (struct) assignment.
-//
-// Arguments:
-//    dst           - The destination node: local var / block node.
-//    srcOrFillVall - The value to assign for CopyBlk, the integer "fill" for InitBlk
-//    isVolatile    - Whether this is a volatile memory operation or not.
-//
-// Return Value:
-//    Returns the newly constructed and initialized block operation.
-//
-GenTree* Compiler::gtNewBlkOpNode(GenTree* dst, GenTree* srcOrFillVal, bool isVolatile)
-{
-    assert(varTypeIsStruct(dst) && (dst->OperIsIndir() || dst->OperIsLocal()));
-
-    bool isCopyBlock = srcOrFillVal->TypeGet() == dst->TypeGet();
-    if (!isCopyBlock) // InitBlk
-    {
-        assert(genActualTypeIsInt(srcOrFillVal) && dst->TypeIs(TYP_STRUCT));
-        if (!srcOrFillVal->IsIntegralConst(0))
-        {
-            srcOrFillVal = gtNewOperNode(GT_INIT_VAL, TYP_INT, srcOrFillVal);
-        }
-    }
-
-    GenTree* result = gtNewAssignNode(dst, srcOrFillVal);
-
-    /* In the case of CpBlk, we want to avoid generating
-    * nodes where the source and destination are the same
-    * because of two reasons, first, is useless, second
-    * it introduces issues in liveness and also copying
-    * memory from an overlapping memory location is
-    * undefined both as per the ECMA standard and also
-    * the memcpy semantics specify that.
-    *
-    * NOTE: In this case we'll only detect the case for addr of a local
-    * and a local itself, any other complex expressions won't be
-    * caught.
-    *
-    * TODO-Cleanup: though having this logic is goodness (i.e. avoids self-assignment
-    * of struct vars very early), it was added because fgInterBlockLocalVarLiveness()
-    * isn't handling self-assignment of struct variables correctly.  This issue may not
-    * surface if struct promotion is ON (which is the case on x86/arm).  But still the
-    * fundamental issue exists that needs to be addressed.
-    */
-    if (isCopyBlock)
-    {
-        GenTree* currSrc = srcOrFillVal;
-        GenTree* currDst = dst;
-
-        if (currSrc->OperIs(GT_LCL_VAR) && currDst->OperIs(GT_LCL_VAR) &&
-            currSrc->AsLclVarCommon()->GetLclNum() == currDst->AsLclVarCommon()->GetLclNum())
-        {
-            result->gtBashToNOP(); // Make this a NOP.
-            return result;
-        }
-    }
-
-    if (isVolatile)
-    {
-        assert(dst->OperIsIndir());
-        dst->gtFlags |= GTF_IND_VOLATILE;
-    }
-
-#ifdef FEATURE_SIMD
-    // If the source is a SIMD/HWI node of SIMD type, then the dst lclvar struct
-    // should be labeled as simd intrinsic related struct. This is done so that
-    // we do not promote the local, thus avoiding conflicting access methods
-    // (fields vs. whole-register).
-    if (varTypeIsSIMD(srcOrFillVal) && srcOrFillVal->OperIs(GT_HWINTRINSIC, GT_CNS_VEC))
-    {
-        // TODO-Cleanup: similar logic already exists in "gtNewAssignNode",
-        // however, it is not enabled for x86. Fix that and delete this code.
-        SetOpLclRelatedToSIMDIntrinsic(dst);
-    }
-#endif // FEATURE_SIMD
-
-    return result;
-}
-
-//------------------------------------------------------------------------
 // gtNewPutArgReg: Creates a new PutArgReg node.
 //
 // Arguments:

diff --git a/src/coreclr/jit/gschecks.cpp b/src/coreclr/jit/gschecks.cpp
index 542946a..9567d80 100644 (file)
--- a/src/coreclr/jit/gschecks.cpp
+++ b/src/coreclr/jit/gschecks.cpp
@@ -411,20 +411,9 @@ void Compiler::gsParamsToShadows()
         LclVarDsc* shadowVarDsc = lvaGetDesc(shadowVarNum);
 
         // Copy some info
-
-        var_types type       = varTypeIsSmall(varDsc->TypeGet()) ? TYP_INT : varDsc->TypeGet();
-        shadowVarDsc->lvType = type;
-
-#ifdef FEATURE_SIMD
-        shadowVarDsc->lvUsedInSIMDIntrinsic = varDsc->lvUsedInSIMDIntrinsic;
-        if (varTypeIsSIMD(varDsc))
-        {
-            CorInfoType simdBaseJitType = varDsc->GetSimdBaseJitType();
-            shadowVarDsc->SetSimdBaseJitType(simdBaseJitType);
-        }
-#endif
+        var_types type            = varTypeIsSmall(varDsc->TypeGet()) ? TYP_INT : varDsc->TypeGet();
+        shadowVarDsc->lvType      = type;
         shadowVarDsc->lvRegStruct = varDsc->lvRegStruct;
-
         shadowVarDsc->SetAddressExposed(varDsc->IsAddressExposed() DEBUGARG(varDsc->GetAddrExposedReason()));
         shadowVarDsc->lvDoNotEnregister = varDsc->lvDoNotEnregister;
 #ifdef DEBUG
@@ -527,7 +516,7 @@ void Compiler::gsParamsToShadows()
         }
 
         GenTree* src = gtNewLclvNode(lclNum, varDsc->TypeGet());
-        src->gtFlags |= GTF_DONT_CSE; // TODO-ASG-Cleanup: delete.
+        src->gtFlags |= GTF_DONT_CSE;
         GenTree* store = gtNewStoreLclVarNode(shadowVarNum, src);
 
         fgEnsureFirstBBisScratch();
@@ -563,7 +552,7 @@ void Compiler::gsParamsToShadows()
                 }
 
                 GenTree* src = gtNewLclVarNode(shadowVarNum);
-                src->gtFlags |= GTF_DONT_CSE; // TODO-ASG-Cleanup: delete.
+                src->gtFlags |= GTF_DONT_CSE;
                 GenTree* store = gtNewStoreLclVarNode(lclNum, src);
 
                 (void)fgNewStmtNearEnd(block, fgMorphTree(store));

diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index e6b3bb4..29297a6 100644 (file)
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -1064,8 +1064,7 @@ GenTree* Compiler::impAssignStruct(GenTree*         dest,
     }
 
     // Return a store node, to be appended.
-    GenTree* storeNode = gtNewBlkOpNode(dest, src);
-
+    GenTree* storeNode = gtNewAssignNode(dest, src);
     return storeNode;
 }
 
@@ -3756,7 +3755,7 @@ GenTree* Compiler::impImportStaticReadOnlyField(CORINFO_FIELD_HANDLE field, CORI
                 // realType is either struct or SIMD
                 var_types      realType  = lvaGetRealType(structTempNum);
                 GenTreeLclVar* structLcl = gtNewLclvNode(structTempNum, realType);
-                impAppendTree(gtNewBlkOpNode(structLcl, gtNewIconNode(0)), CHECK_SPILL_NONE, impCurStmtDI);
+                impAppendTree(gtNewAssignNode(structLcl, gtNewIconNode(0)), CHECK_SPILL_NONE, impCurStmtDI);
 
                 return gtNewLclvNode(structTempNum, realType);
             }
@@ -8751,17 +8750,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                         if (fgVarNeedsExplicitZeroInit(lclNum, bbInALoop, bbIsReturn))
                         {
                             // Append a tree to zero-out the temp
-                            GenTree* newObjDst = gtNewLclvNode(lclNum, lclDsc->TypeGet());
-                            GenTree* newObjInit;
-                            if (lclDsc->TypeGet() == TYP_STRUCT)
-                            {
-                                newObjInit = gtNewBlkOpNode(newObjDst, gtNewIconNode(0));
-                            }
-                            else
-                            {
-                                newObjInit = gtNewAssignNode(newObjDst, gtNewZeroConNode(lclDsc->TypeGet()));
-                            }
-                            impAppendTree(newObjInit, CHECK_SPILL_NONE, impCurStmtDI);
+                            GenTree* newObjInit =
+                                gtNewZeroConNode((lclDsc->TypeGet() == TYP_STRUCT) ? TYP_INT : lclDsc->TypeGet());
+
+                            impAssignTempGen(lclNum, newObjInit, CHECK_SPILL_NONE);
                         }
                         else
                         {
@@ -10486,7 +10478,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 op1 = impPopStack().val;
                 op1 = gtNewLoadValueNode(layout, op1);
                 op2 = gtNewIconNode(0);
-                op1 = gtNewBlkOpNode(op1, op2, (prefixFlags & PREFIX_VOLATILE) != 0);
+                op1 = gtNewAssignNode(op1, op2);
                 goto SPILL_APPEND;
             }
 
@@ -10516,9 +10508,21 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     }
 
                     ClassLayout* layout = typGetBlkLayout(static_cast<unsigned>(op3->AsIntConCommon()->IconValue()));
-                    op1                 = gtNewLoadValueNode(layout, op1, indirFlags);
-                    op2                 = opcode == CEE_INITBLK ? op2 : gtNewLoadValueNode(layout, op2, indirFlags);
-                    op1                 = gtNewBlkOpNode(op1, op2, (indirFlags & GTF_IND_VOLATILE) != 0);
+
+                    if (opcode == CEE_INITBLK)
+                    {
+                        if (!op2->IsIntegralConst(0))
+                        {
+                            op2 = gtNewOperNode(GT_INIT_VAL, TYP_INT, op2);
+                        }
+                    }
+                    else
+                    {
+                        op2 = gtNewLoadValueNode(layout, op2, indirFlags);
+                    }
+
+                    op1 = gtNewLoadValueNode(layout, op1, indirFlags);
+                    op1 = gtNewAssignNode(op1, op2);
                 }
                 else
                 {
@@ -10568,7 +10572,7 @@ void Compiler::impImportBlockCode(BasicBlock* block)
 
                 op1 = gtNewLoadValueNode(layout, op1);
                 op2 = gtNewLoadValueNode(layout, op2);
-                op1 = gtNewBlkOpNode(op1, op2, ((prefixFlags & PREFIX_VOLATILE) != 0));
+                op1 = gtNewAssignNode(op1, op2);
                 goto SPILL_APPEND;
             }
 

diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp
index ce7d2f2..3a84278 100644 (file)
--- a/src/coreclr/jit/importercalls.cpp
+++ b/src/coreclr/jit/importercalls.cpp
@@ -2136,7 +2136,7 @@ GenTree* Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig)
     src->gtGetOp1()->AsIntCon()->gtTargetHandle = THT_InitializeArrayIntrinsics;
 #endif
 
-    return gtNewBlkOpNode(dst, src);
+    return gtNewAssignNode(dst, src);
 }
 
 GenTree* Compiler::impCreateSpanIntrinsic(CORINFO_SIG_INFO* sig)

diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp
index ae35fc7..d9ce7d1 100644 (file)
--- a/src/coreclr/jit/lclvars.cpp
+++ b/src/coreclr/jit/lclvars.cpp
@@ -2935,19 +2935,6 @@ void Compiler::lvaSetStruct(unsigned varNum, ClassLayout* layout, bool unsafeVal
             }
 #endif // FEATURE_IMPLICIT_BYREFS
 
-#if FEATURE_SIMD
-            if (varTypeIsSIMD(layout->GetType()))
-            {
-                CorInfoType simdBaseJitType = CORINFO_TYPE_UNDEF;
-                impNormStructType(layout->GetClassHandle(), &simdBaseJitType);
-
-                if (simdBaseJitType != CORINFO_TYPE_UNDEF)
-                {
-                    assert(varTypeIsSIMD(varDsc));
-                    varDsc->SetSimdBaseJitType(simdBaseJitType);
-                }
-            }
-#endif // FEATURE_SIMD
             // For structs that are small enough, we check and set HFA element type
             if (GlobalJitOptions::compFeatureHfa && (layout->GetSize() <= MAX_PASS_MULTIREG_BYTES))
             {
@@ -2969,9 +2956,6 @@ void Compiler::lvaSetStruct(unsigned varNum, ClassLayout* layout, bool unsafeVal
     }
     else
     {
-#if FEATURE_SIMD
-        assert(!varTypeIsSIMD(varDsc) || (varDsc->GetSimdBaseType() != TYP_UNKNOWN));
-#endif // FEATURE_SIMD
         assert(ClassLayout::AreCompatible(varDsc->GetLayout(), layout));
         // Inlining could replace a canon struct type with an exact one.
         varDsc->SetLayout(layout);
@@ -3792,19 +3776,6 @@ void Compiler::lvaSortByRefCount()
 #endif
 }
 
-#ifdef FEATURE_SIMD
-var_types LclVarDsc::GetSimdBaseType() const
-{
-    CorInfoType simdBaseJitType = GetSimdBaseJitType();
-
-    if (simdBaseJitType == CORINFO_TYPE_UNDEF)
-    {
-        return TYP_UNKNOWN;
-    }
-    return JitType2PreciseVarType(simdBaseJitType);
-}
-#endif // FEATURE_SIMD
-
 //------------------------------------------------------------------------
 // lvExactSize: Get the exact size of the type of this local.
 //

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
index 581f724..0272159 100644 (file)
--- a/src/coreclr/jit/morph.cpp
+++ b/src/coreclr/jit/morph.cpp
@@ -4050,7 +4050,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg)
 
     // Copy the valuetype to the temp
     GenTree* dest    = gtNewLclvNode(tmp, lvaGetDesc(tmp)->TypeGet());
-    GenTree* copyBlk = gtNewBlkOpNode(dest, argx);
+    GenTree* copyBlk = gtNewAssignNode(dest, argx);
     copyBlk          = fgMorphCopyBlock(copyBlk);
 
     call->gtArgs.SetTemp(arg, tmp);
@@ -7505,18 +7505,9 @@ void Compiler::fgMorphRecursiveFastTailCallIntoLoop(BasicBlock* block, GenTreeCa
             bool      hadSuppressedInit  = varDsc->lvSuppressedZeroInit;
             if ((info.compInitMem && (isUserLocal || structWithGCFields)) || hadSuppressedInit)
             {
-                GenTree* lcl  = gtNewLclvNode(varNum, lclType);
-                GenTree* init = nullptr;
-                if (lclType == TYP_STRUCT)
-                {
-                    init = gtNewBlkOpNode(lcl, gtNewIconNode(0));
-                    init = fgMorphInitBlock(init);
-                }
-                else
-                {
-                    GenTree* zero = gtNewZeroConNode(lclType);
-                    init          = gtNewAssignNode(lcl, zero);
-                }
+                GenTree*   lcl      = gtNewLclvNode(varNum, lclType);
+                GenTree*   zero     = gtNewZeroConNode((lclType == TYP_STRUCT) ? TYP_INT : lclType);
+                GenTree*   init     = gtNewAssignNode(lcl, zero);
                 Statement* initStmt = gtNewStmt(init, callDI);
                 fgInsertStmtBefore(block, lastStmt, initStmt);
             }

diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp
index 80ef539..e376241 100644 (file)
--- a/src/coreclr/jit/objectalloc.cpp
+++ b/src/coreclr/jit/objectalloc.cpp
@@ -530,7 +530,7 @@ unsigned int ObjectAllocator::MorphAllocObjNodeIntoStackAlloc(GenTreeAllocObj* a
         //------------------------------------------------------------------------
 
         GenTree* tree = comp->gtNewLclvNode(lclNum, TYP_STRUCT);
-        tree          = comp->gtNewBlkOpNode(tree, comp->gtNewIconNode(0));
+        tree          = comp->gtNewAssignNode(tree, comp->gtNewIconNode(0));
 
         Statement* newStmt = comp->gtNewStmt(tree);
 

diff --git a/src/coreclr/jit/promotiondecomposition.cpp b/src/coreclr/jit/promotiondecomposition.cpp
index 4239694..616b419 100644 (file)
--- a/src/coreclr/jit/promotiondecomposition.cpp
+++ b/src/coreclr/jit/promotiondecomposition.cpp
@@ -724,8 +724,7 @@ private:
     //
     void FinalizeInit(DecompositionStatementList* statements)
     {
-        GenTree* cns         = m_src->OperIsInitVal() ? m_src->gtGetOp1() : m_src;
-        uint8_t  initPattern = GetInitPattern();
+        uint8_t initPattern = GetInitPattern();
 
         for (int i = 0; i < m_entries.Height(); i++)
         {
@@ -742,7 +741,7 @@ private:
         RemainderStrategy remainderStrategy = DetermineRemainderStrategy();
         if (remainderStrategy.Type == RemainderStrategy::FullBlock)
         {
-            GenTree* asg = m_compiler->gtNewBlkOpNode(m_dst, cns);
+            GenTree* asg = m_compiler->gtNewAssignNode(m_dst, m_src);
             statements->AddStatement(asg);
         }
         else if (remainderStrategy.Type == RemainderStrategy::Primitive)
@@ -962,7 +961,7 @@ private:
         // that it's best to do it last.
         if ((remainderStrategy.Type == RemainderStrategy::FullBlock) && m_srcInvolvesReplacements)
         {
-            statements->AddStatement(m_compiler->gtNewBlkOpNode(m_dst, m_src));
+            statements->AddStatement(m_compiler->gtNewAssignNode(m_dst, m_src));
 
             if (m_src->OperIs(GT_LCL_VAR, GT_LCL_FLD))
             {
@@ -1043,7 +1042,7 @@ private:
 
         if ((remainderStrategy.Type == RemainderStrategy::FullBlock) && !m_srcInvolvesReplacements)
         {
-            statements->AddStatement(m_compiler->gtNewBlkOpNode(m_dst, m_src));
+            statements->AddStatement(m_compiler->gtNewAssignNode(m_dst, m_src));
         }
 
         if (remainderStrategy.Type == RemainderStrategy::Primitive)

diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp
index 5136b4a..580846d 100644 (file)
--- a/src/coreclr/jit/simdashwintrinsic.cpp
+++ b/src/coreclr/jit/simdashwintrinsic.cpp
@@ -2206,7 +2206,7 @@ GenTree* Compiler::impSimdAsHWIntrinsicSpecial(NamedIntrinsic       intrinsic,
     {
         assert(copyBlkSrc != nullptr);
         GenTree* dest    = gtNewLoadValueNode(simdType, copyBlkDst);
-        GenTree* retNode = gtNewBlkOpNode(dest, copyBlkSrc);
+        GenTree* retNode = gtNewAssignNode(dest, copyBlkSrc);
 
         return retNode;
     }