Morph ASG(ONE_FLD_STRUCT, CALL) using BITCAST

This changes the representation we have for the special case
of assignments from calls that return a struct with one field
to a promoted local of that struct type, making it more normal
and simplifying the model for SSA and value numbering.

In the past, the optimization phases treated this case specially
and "skipped" the outer struct local in relevant cases. Instead,
I propose a new form for this assignment: ASG(FLD, BITCAST(CALL)),
that gets rid of this special-casing, and introduces the useful
invariant that if a node has an SSA name, then it always represents
a def or use of the corresponding SSA local.

After optimizations have done their work, lowering removes the new
bitcasts as they do not have meaningful value in the backend.

There are some downsides to this representation. For example, if
we decide to stop retyping helper calls in the importer, VN will
start giving BITCAST VNs to locals assigned values produced by
important helpers such as TypeHandleToRuntimeTypeHandle.

It makes the IR a bit larger, and, perhaps more importantly, more
sparse. I can measure a 0.003% increase in memory consumption when
CG-ing x64 CoreLib with this change. There are some TP benefits
from the reduced checks too, they may offset this regression.

It seems to me that the benefits outweigh the drawbacks in this case.

src/coreclr/jit/copyprop.cpp

@@ -257,19 +257,13 @@ void Compiler::optCopyProp(Statement* stmt, GenTreeLclVarCommon* tree, unsigned
 //
 // Returns:
 //    - lclNum if the local is participating in SSA;
-//    - fieldLclNum if the parent local can be replaced by its only field;
 //    - BAD_VAR_NUM otherwise.
 //
 unsigned Compiler::optIsSsaLocal(GenTreeLclVarCommon* lclNode)
 {
     unsigned   lclNum = lclNode->GetLclNum();
     LclVarDsc* varDsc = lvaGetDesc(lclNum);
 
-    if (!lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(this))
-    {
-        lclNum = varDsc->lvFieldLclStart;
-    }
-
     if (!lvaInSsa(lclNum))
     {
         return BAD_VAR_NUM;

src/coreclr/jit/earlyprop.cpp

@@ -374,8 +374,7 @@ GenTree* Compiler::optPropGetValueRec(unsigned lclNum, unsigned ssaNum, optPropK
 
         GenTree* treeRhs = ssaDefAsg->gtGetOp2();
 
-        if (treeRhs->OperIsScalarLocal() && lvaInSsa(treeRhs->AsLclVarCommon()->GetLclNum()) &&
-            treeRhs->AsLclVarCommon()->HasSsaName())
+        if (treeRhs->OperIsScalarLocal() && treeRhs->AsLclVarCommon()->HasSsaName())
         {
             // Recursively track the Rhs
             unsigned rhsLclNum = treeRhs->AsLclVarCommon()->GetLclNum();

src/coreclr/jit/gentree.cpp

@@ -23307,6 +23307,30 @@ unsigned GenTreeHWIntrinsic::GetResultOpNumForFMA(GenTree* use, GenTree* op1, Ge
 }
 #endif // TARGET_XARCH && FEATURE_HW_INTRINSICS
 
+//------------------------------------------------------------------------
+// HasSsaName: Does this local node have an SSA name?
+//
+// Return Value:
+//    Whether this node's SSA name is not RESERVED_SSA_NUM.
+//
+// Notes:
+//    If the node has an SSA name, it will always represent a local that
+//    participates in SSA. Thus, "HasSsaName" implies "lvaIsInSsa". The
+//    opposite is not true - a node may be in an unreachable block not
+//    visited by the renamer.
+//
+bool GenTreeLclVarCommon::HasSsaName() const
+{
+    bool hasSsaName = GetSsaNum() != SsaConfig::RESERVED_SSA_NUM;
+
+    if (hasSsaName)
+    {
+        assert(JitTls::GetCompiler()->lvaInSsa(GetLclNum()));
+    }
+
+    return hasSsaName;
+}
+
 unsigned GenTreeLclFld::GetSize() const
 {
     return TypeIs(TYP_STRUCT) ? GetLayout()->GetSize() : genTypeSize(TypeGet());

src/coreclr/jit/gentree.h

@@ -3550,10 +3550,7 @@ struct GenTreeLclVarCommon : public GenTreeUnOp
         _gtSsaNum = ssaNum;
     }
 
-    bool HasSsaName()
-    {
-        return (GetSsaNum() != SsaConfig::RESERVED_SSA_NUM);
-    }
+    bool HasSsaName() const;
 
 #if DEBUGGABLE_GENTREE
     GenTreeLclVarCommon() : GenTreeUnOp()

src/coreclr/jit/lower.cpp

@@ -246,6 +246,9 @@ GenTree* Lowering::LowerNode(GenTree* node)
             LowerRet(node->AsUnOp());
             break;
 
+        case GT_BITCAST:
+            return LowerBitcast(node->AsUnOp());
+
         case GT_RETURNTRAP:
             ContainCheckReturnTrap(node->AsOp());
             break;
@@ -3360,6 +3363,44 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
     ContainCheckRet(ret);
 }
 
+//------------------------------------------------------------------------
+// LowerBitcast: Lower a GT_BITCAST node.
+//
+// Will remove bitcasts that do not move between different register files.
+// Such bitcasts are generated in block morphing for assignments of struct
+// locals that "can be replaced with their one field" to calls. This makes
+// the frontend (SSA) simpler, but here, in the backend, they end up as
+// unnecessary no-op BITCAST(type -> type) nodes, so we remove them.
+//
+// Arguments:
+//    bitcast - the bitcast node to lower
+//
+// Return Value:
+//    The next node to lower.
+//
+GenTree* Lowering::LowerBitcast(GenTreeUnOp* bitcast)
+{
+    GenTree* nextNode = bitcast->gtNext;
+    GenTree* src      = bitcast->gtGetOp1();
+
+    if (bitcast->TypeGet() == src->TypeGet())
+    {
+        LIR::Use bitcastUse;
+        if (BlockRange().TryGetUse(bitcast, &bitcastUse))
+        {
+            bitcastUse.ReplaceWith(src);
+        }
+        else
+        {
+            src->SetUnusedValue();
+        }
+
+        BlockRange().Remove(bitcast);
+    }
+
+    return nextNode;
+}
+
 //----------------------------------------------------------------------------------------------
 // LowerStoreLocCommon: platform idependent part of local var or field store lowering.
 //
@@ -3814,6 +3855,10 @@ void Lowering::LowerCallStruct(GenTreeCall* call)
         GenTree* user = callUse.User();
         switch (user->OperGet())
         {
+            case GT_BITCAST:
+                assert(genTypeSize(user) == genTypeSize(call));
+                break;
+
             case GT_RETURN:
             case GT_STORE_LCL_VAR:
             case GT_STORE_BLK:

src/coreclr/jit/lower.h

@@ -136,6 +136,7 @@ class Lowering final : public Phase
     GenTreeCC* LowerNodeCC(GenTree* node, GenCondition condition);
     void LowerJmpMethod(GenTree* jmp);
     void LowerRet(GenTreeUnOp* ret);
+    GenTree* LowerBitcast(GenTreeUnOp* node);
     void LowerStoreLocCommon(GenTreeLclVarCommon* lclVar);
     void LowerRetStruct(GenTreeUnOp* ret);
     void LowerRetSingleRegStructLclVar(GenTreeUnOp* ret);

src/coreclr/jit/lsrabuild.cpp

@@ -3523,11 +3523,11 @@ int LinearScan::BuildStoreLoc(GenTreeLclVarCommon* storeLoc)
     else if (op1->isContained() && op1->OperIs(GT_BITCAST))
     {
         GenTree*     bitCastSrc   = op1->gtGetOp1();
-        RegisterType registerType = bitCastSrc->TypeGet();
+        RegisterType registerType = regType(getDefType(bitCastSrc));
         singleUseRef              = BuildUse(bitCastSrc, allRegs(registerType));
 
         Interval* srcInterval = singleUseRef->getInterval();
-        assert(srcInterval->registerType == registerType);
+        assert(srcInterval->registerType == getDefType(bitCastSrc));
         srcCount = 1;
     }
 #ifndef TARGET_64BIT

src/coreclr/jit/morphblock.cpp

@@ -54,6 +54,7 @@ class MorphInitBlockHelper
         OneAsgBlock,
         StructBlock,
         SkipCallSrc,
+        BitcastCallSrc,
         SkipMultiRegIntrinsicSrc,
         Nop
     };
@@ -806,15 +807,33 @@ void MorphCopyBlockHelper::TrySpecialCases()
                     m_asg->gtOp1 = lclVar;
                 }
             }
+
             if (m_dst->OperIs(GT_LCL_VAR))
             {
                 LclVarDsc* varDsc = m_comp->lvaGetDesc(m_dst->AsLclVar());
                 if (varTypeIsStruct(varDsc) && varDsc->CanBeReplacedWithItsField(m_comp))
                 {
+                    assert(m_comp->fgGlobalMorph);
+
+                    JITDUMP("Morphing a single reg call return into BITCAST assigned to the promoted field\n");
+                    m_transformationDecision = BlockTransformation::BitcastCallSrc;
+
+                    unsigned   fieldNum  = varDsc->lvFieldLclStart;
+                    LclVarDsc* fieldDsc  = m_comp->lvaGetDesc(fieldNum);
+                    var_types  fieldType = fieldDsc->TypeGet();
+                    assert(!varTypeIsStruct(fieldType));
+
                     m_dst->gtFlags |= GTF_DONT_CSE;
-                    JITDUMP("Not morphing a single reg call return\n");
-                    m_transformationDecision = BlockTransformation::SkipCallSrc;
-                    m_result                 = m_asg;
+                    m_dst->ChangeType(fieldType);
+                    m_dst->AsLclVar()->SetLclNum(fieldNum);
+
+                    assert(fieldDsc->lvNormalizeOnLoad() || !varTypeIsSmall(fieldDsc));
+                    GenTree* bitcast = m_comp->gtNewOperNode(GT_BITCAST, genActualType(fieldType), m_src);
+                    m_src            = bitcast;
+                    m_asg->gtOp2     = bitcast;
+
+                    m_asg->ChangeType(fieldType);
+                    m_result = m_asg;
                 }
             }
         }

src/coreclr/jit/optimizer.cpp

@@ -7264,7 +7264,7 @@ void Compiler::optHoistLoopBlocks(unsigned loopNum, ArrayStack<BasicBlock*>* blo
                 // To be invariant a LclVar node must not be the LHS of an assignment ...
                 bool isInvariant = !user->OperIs(GT_ASG) || (user->AsOp()->gtGetOp1() != tree);
                 // and the variable must be in SSA ...
-                isInvariant = isInvariant && m_compiler->lvaInSsa(lclNum) && lclVar->HasSsaName();
+                isInvariant = isInvariant && lclVar->HasSsaName();
                 // and the SSA definition must be outside the loop we're hoisting from ...
                 isInvariant = isInvariant &&
                               !m_compiler->optLoopTable[m_loopNum].lpContains(
@@ -8455,7 +8455,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
                     {
                         // If it's a local byref for which we recorded a value number, use that...
                         GenTreeLclVar* argLcl = arg->AsLclVar();
-                        if (lvaInSsa(argLcl->GetLclNum()) && argLcl->HasSsaName())
+                        if (argLcl->HasSsaName())
                         {
                             ValueNum argVN =
                                 lvaTable[argLcl->GetLclNum()].GetPerSsaData(argLcl->GetSsaNum())->m_vnPair.GetLiberal();
@@ -8533,7 +8533,7 @@ bool Compiler::optComputeLoopSideEffectsOfBlock(BasicBlock* blk)
                     if (rhsVN != ValueNumStore::NoVN)
                     {
                         rhsVN = vnStore->VNNormalValue(rhsVN);
-                        if (lvaInSsa(lhsLcl->GetLclNum()) && lhsLcl->HasSsaName())
+                        if (lhsLcl->HasSsaName())
                         {
                             lvaTable[lhsLcl->GetLclNum()]
                                 .GetPerSsaData(lhsLcl->GetSsaNum())

src/coreclr/jit/rangecheck.cpp

@@ -454,11 +454,7 @@ LclSsaVarDsc* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse)
         return nullptr;
     }
 
-    LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclUse);
-    if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
-    {
-        varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart);
-    }
+    LclVarDsc*    varDsc = m_pCompiler->lvaGetDesc(lclUse);
     LclSsaVarDsc* ssaDef = varDsc->GetPerSsaData(ssaNum);
 
     // RangeCheck does not care about uninitialized variables.
@@ -493,10 +489,6 @@ LclSsaVarDsc* RangeCheck::GetSsaDefAsg(GenTreeLclVarCommon* lclUse)
 UINT64 RangeCheck::HashCode(unsigned lclNum, unsigned ssaNum)
 {
     LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum);
-    if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
-    {
-        lclNum = varDsc->lvFieldLclStart;
-    }
     assert(ssaNum != SsaConfig::RESERVED_SSA_NUM);
     return UINT64(lclNum) << 32 | ssaNum;
 }
@@ -563,11 +555,7 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
         return;
     }
 
-    LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl);
-    if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
-    {
-        varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart);
-    }
+    LclVarDsc*    varDsc      = m_pCompiler->lvaGetDesc(lcl);
     LclSsaVarDsc* ssaData     = varDsc->GetPerSsaData(lcl->GetSsaNum());
     ValueNum      normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair);
     MergeEdgeAssertions(normalLclVN, assertions, pRange);

src/coreclr/jit/ssabuilder.cpp

@@ -745,13 +745,6 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block)
         unsigned   lclNum = lclNode->GetLclNum();
         LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum);
 
-        if (!m_pCompiler->lvaInSsa(lclNum) && varDsc->CanBeReplacedWithItsField(m_pCompiler))
-        {
-            lclNum = varDsc->lvFieldLclStart;
-            varDsc = m_pCompiler->lvaGetDesc(lclNum);
-            assert(isFullDef);
-        }
-
         if (m_pCompiler->lvaInSsa(lclNum))
         {
             // Promoted variables are not in SSA, only their fields are.

src/coreclr/jit/valuenum.cpp

@@ -8188,24 +8188,6 @@ void Compiler::fgValueNumberBlockAssignment(GenTree* tree)
 
             fgValueNumberLocalStore(tree, lclVarTree, offset, storeSize, rhsVNPair);
         }
-        else if (lclVarTree->HasSsaName())
-        {
-            // The local wasn't in SSA, the tree is still an SSA def. There is only one
-            // case when this can happen - a promoted "CanBeReplacedWithItsField" struct.
-            assert((lhs == lclVarTree) && rhs->IsCall() && isEntire);
-            assert(lhsVarDsc->CanBeReplacedWithItsField(this));
-            // Give a new, unique, VN to the field.
-            LclVarDsc*    fieldVarDsc    = lvaGetDesc(lhsVarDsc->lvFieldLclStart);
-            LclSsaVarDsc* fieldVarSsaDsc = fieldVarDsc->GetPerSsaData(lclVarTree->GetSsaNum());
-            ValueNum      newUniqueVN    = vnStore->VNForExpr(compCurBB, fieldVarDsc->TypeGet());
-
-            fieldVarSsaDsc->m_vnPair.SetBoth(newUniqueVN);
-
-            JITDUMP("Tree [%06u] assigned VN to the only field V%02u/%u of promoted struct V%02u: new uniq ",
-                    dspTreeID(tree), lhsVarDsc->lvFieldLclStart, lclVarTree->GetSsaNum(), lhsLclNum);
-            JITDUMPEXEC(vnPrint(newUniqueVN, 1));
-            JITDUMP("\n");
-        }
         else if (lhsVarDsc->IsAddressExposed())
         {
             fgMutateAddressExposedLocal(tree DEBUGARG("INITBLK/COPYBLK - address-exposed local"));
@@ -8324,7 +8306,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
                 {
                     unsigned lclNum = lclFld->GetLclNum();
 
-                    if (!lvaInSsa(lclFld->GetLclNum()) || !lclFld->HasSsaName())
+                    if (!lclFld->HasSsaName())
                     {
                         lclFld->gtVNPair.SetBoth(vnStore->VNForExpr(compCurBB, lclFld->TypeGet()));
                     }