Track single def locals in importer (#21251)

* Defer setting lvSingleDef until lvaMarkLocalVars

Since this information isn't used early on in the jit, defer setting this bit
until later.

* Track single-def locals in the importer

and use this information to enable type updates during late devirtualization.
This gets some cases where the exact type materializes late (perhaps via a
chain of copies).

diff --git a/src/jit/compiler.h b/src/jit/compiler.h
index 3ac8b9c..986f0ab 100644 (file)
--- a/src/jit/compiler.h
+++ b/src/jit/compiler.h
@@ -359,8 +359,11 @@ public:
 #if OPT_BOOL_OPS
     unsigned char lvIsBoolean : 1; // set if variable is boolean
 #endif
+    unsigned char lvSingleDef : 1; // variable has a single def
+                                   // before lvaMarkLocalVars: identifies ref type locals that can get type updates
+                                   // after lvaMarkLocalVars: identifies locals that are suitable for optAddCopies
+
 #if ASSERTION_PROP
-    unsigned char lvSingleDef : 1;    // variable has a single def
     unsigned char lvDisqualify : 1;   // variable is no longer OK for add copy optimization
     unsigned char lvVolatileHint : 1; // hint for AssertionProp
 #endif

diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp
index 57948cf..41a88d9 100644 (file)
--- a/src/jit/flowgraph.cpp
+++ b/src/jit/flowgraph.cpp
@@ -4944,6 +4944,34 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
     {
         fgAdjustForAddressExposedOrWrittenThis();
     }
+
+    // Now that we've seen the IL, set lvSingleDef for root method
+    // locals.
+    //
+    // We could also do this for root method arguments but single-def
+    // arguments are set by the caller and so we don't know anything
+    // about the possible values or types.
+    //
+    // For inlinees we do this over in impInlineFetchLocal and
+    // impInlineFetchArg (here args are included as we somtimes get
+    // new information about the types of inlinee args).
+    if (!isInlining)
+    {
+        const unsigned firstLcl = info.compArgsCount;
+        const unsigned lastLcl  = firstLcl + info.compMethodInfo->locals.numArgs;
+        for (unsigned lclNum = firstLcl; lclNum < lastLcl; lclNum++)
+        {
+            LclVarDsc* lclDsc = lvaGetDesc(lclNum);
+            assert(lclDsc->lvSingleDef == 0);
+            // could restrict this to TYP_REF
+            lclDsc->lvSingleDef = !lclDsc->lvHasMultipleILStoreOp && !lclDsc->lvHasLdAddrOp;
+
+            if (lclDsc->lvSingleDef)
+            {
+                JITDUMP("Marked V%02u as a single def local\n", lclNum);
+            }
+        }
+    }
 }
 
 #ifdef _PREFAST_
@@ -5876,6 +5904,9 @@ void Compiler::fgFindBasicBlocks()
                 // out we can prove the method returns a more specific type.
                 if (info.compRetType == TYP_REF)
                 {
+                    lvaTable[lvaInlineeReturnSpillTemp].lvSingleDef = 1;
+                    JITDUMP("Marked V%02u as a single def temp\n", lvaInlineeReturnSpillTemp);
+
                     CORINFO_CLASS_HANDLE retClassHnd = impInlineInfo->inlineCandidateInfo->methInfo.args.retTypeClass;
                     if (retClassHnd != nullptr)
                     {
@@ -22442,6 +22473,31 @@ Compiler::fgWalkResult Compiler::fgLateDevirtualization(GenTree** pTree, fgWalkD
             comp->impDevirtualizeCall(call, &method, &methodFlags, &context, nullptr);
         }
     }
+    else if (tree->OperGet() == GT_ASG)
+    {
+        // If we're assigning to a ref typed local that has one definition,
+        // we may be able to sharpen the type for the local.
+        GenTree* lhs = tree->gtGetOp1()->gtEffectiveVal();
+
+        if ((lhs->OperGet() == GT_LCL_VAR) && (lhs->TypeGet() == TYP_REF))
+        {
+            const unsigned lclNum = lhs->gtLclVarCommon.gtLclNum;
+            LclVarDsc*     lcl    = comp->lvaGetDesc(lclNum);
+
+            if (lcl->lvSingleDef)
+            {
+                GenTree*             rhs       = tree->gtGetOp2();
+                bool                 isExact   = false;
+                bool                 isNonNull = false;
+                CORINFO_CLASS_HANDLE newClass  = comp->gtGetClassHandle(rhs, &isExact, &isNonNull);
+
+                if (newClass != NO_CLASS_HANDLE)
+                {
+                    comp->lvaUpdateClass(lclNum, newClass, isExact);
+                }
+            }
+        }
+    }
 
     return WALK_CONTINUE;
 }

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 371d277..cefb2fa 100644 (file)
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -2255,6 +2255,9 @@ bool Compiler::impSpillStackEntry(unsigned level,
     // If temp is newly introduced and a ref type, grab what type info we can.
     if (isNewTemp && (lvaTable[tnum].lvType == TYP_REF))
     {
+        assert(lvaTable[tnum].lvSingleDef == 0);
+        lvaTable[tnum].lvSingleDef = 1;
+        JITDUMP("Marked V%02u as a single def temp\n", tnum);
         CORINFO_CLASS_HANDLE stkHnd = verCurrentState.esStack[level].seTypeInfo.GetClassHandle();
         lvaSetClass(tnum, tree, stkHnd);
 
@@ -6052,9 +6055,11 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken)
         {
             // When optimizing, use a new temp for each box operation
             // since we then know the exact class of the box temp.
-            impBoxTemp                  = lvaGrabTemp(true DEBUGARG("Single-def Box Helper"));
-            lvaTable[impBoxTemp].lvType = TYP_REF;
-            const bool isExact          = true;
+            impBoxTemp                       = lvaGrabTemp(true DEBUGARG("Single-def Box Helper"));
+            lvaTable[impBoxTemp].lvType      = TYP_REF;
+            lvaTable[impBoxTemp].lvSingleDef = 1;
+            JITDUMP("Marking V%02u as a single def local\n", impBoxTemp);
+            const bool isExact = true;
             lvaSetClass(impBoxTemp, pResolvedToken->hClass, isExact);
         }
 
@@ -10559,6 +10564,10 @@ GenTree* Compiler::impCastClassOrIsInstToTree(GenTree*                op1,
     //
     // See also gtGetHelperCallClassHandle where we make the same
     // determination for the helper call variants.
+    LclVarDsc* lclDsc = lvaGetDesc(tmp);
+    assert(lclDsc->lvSingleDef == 0);
+    lclDsc->lvSingleDef = 1;
+    JITDUMP("Marked V%02u as a single def temp\n", tmp);
     lvaSetClass(tmp, pResolvedToken->hClass);
     return gtNewLclvNode(tmp, TYP_REF);
 #endif
@@ -11214,14 +11223,14 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     // We should have seen a stloc in our IL prescan.
                     assert(lvaTable[lclNum].lvHasILStoreOp);
 
-                    const bool isSingleILStoreLocal =
-                        !lvaTable[lclNum].lvHasMultipleILStoreOp && !lvaTable[lclNum].lvHasLdAddrOp;
+                    // Is there just one place this local is defined?
+                    const bool isSingleDefLocal = lvaTable[lclNum].lvSingleDef;
 
                     // Conservative check that there is just one
                     // definition that reaches this store.
                     const bool hasSingleReachingDef = (block->bbStackDepthOnEntry() == 0);
 
-                    if (isSingleILStoreLocal && hasSingleReachingDef)
+                    if (isSingleDefLocal && hasSingleReachingDef)
                     {
                         lvaUpdateClass(lclNum, op1, clsHnd);
                     }
@@ -13085,6 +13094,9 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     // Propagate type info to the temp from the stack and the original tree
                     if (type == TYP_REF)
                     {
+                        assert(lvaTable[tmpNum].lvSingleDef == 0);
+                        lvaTable[tmpNum].lvSingleDef = 1;
+                        JITDUMP("Marked V%02u as a single def local\n", tmpNum);
                         lvaSetClass(tmpNum, tree, tiRetVal.GetClassHandle());
                     }
                 }
@@ -13801,6 +13813,10 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                         // without exhaustive walk over all expressions.
 
                         impAssignTempGen(lclNum, op1, (unsigned)CHECK_SPILL_NONE);
+
+                        assert(lvaTable[lclNum].lvSingleDef == 0);
+                        lvaTable[lclNum].lvSingleDef = 1;
+                        JITDUMP("Marked V%02u as a single def local\n", lclNum);
                         lvaSetClass(lclNum, resolvedToken.hClass, true /* is Exact */);
 
                         newObjThisPtr = gtNewLclvNode(lclNum, TYP_REF);
@@ -19105,6 +19121,14 @@ unsigned Compiler::impInlineFetchLocal(unsigned lclNum DEBUGARG(const char* reas
         // signature and pass in a more precise type.
         if (lclTyp == TYP_REF)
         {
+            assert(lvaTable[tmpNum].lvSingleDef == 0);
+
+            lvaTable[tmpNum].lvSingleDef = !inlineeLocal.lclHasMultipleStlocOp && !inlineeLocal.lclHasLdlocaOp;
+            if (lvaTable[tmpNum].lvSingleDef)
+            {
+                JITDUMP("Marked V%02u as a single def temp\n", tmpNum);
+            }
+
             lvaSetClass(tmpNum, inlineeLocal.lclVerTypeInfo.GetClassHandleForObjRef());
         }
 
@@ -19296,6 +19320,9 @@ GenTree* Compiler::impInlineFetchArg(unsigned lclNum, InlArgInfo* inlArgInfo, In
                     // If the arg can't be modified in the method
                     // body, use the type of the value, if
                     // known. Otherwise, use the declared type.
+                    assert(lvaTable[tmpNum].lvSingleDef == 0);
+                    lvaTable[tmpNum].lvSingleDef = 1;
+                    JITDUMP("Marked V%02u as a single def temp\n", tmpNum);
                     lvaSetClass(tmpNum, argInfo.argNode, lclInfo.lclVerTypeInfo.GetClassHandleForObjRef());
                 }
                 else

diff --git a/src/jit/lclvars.cpp b/src/jit/lclvars.cpp
index a8ffe7c..9217fe7 100644 (file)
--- a/src/jit/lclvars.cpp
+++ b/src/jit/lclvars.cpp
@@ -387,11 +387,7 @@ void Compiler::lvaInitThisPtr(InitVarDscInfo* varDscInfo)
     if (!info.compIsStatic)
     {
         varDsc->lvIsParam = 1;
-#if ASSERTION_PROP
-        varDsc->lvSingleDef = 1;
-#endif
-
-        varDsc->lvIsPtr = 1;
+        varDsc->lvIsPtr   = 1;
 
         lvaArg0Var = info.compThisArg = varDscInfo->varNum;
         noway_assert(info.compThisArg == 0);
@@ -474,9 +470,7 @@ void Compiler::lvaInitRetBuffArg(InitVarDscInfo* varDscInfo)
         varDsc->lvType      = TYP_BYREF;
         varDsc->lvIsParam   = 1;
         varDsc->lvIsRegArg  = 1;
-#if ASSERTION_PROP
-        varDsc->lvSingleDef = 1;
-#endif
+
         if (hasFixedRetBuffReg())
         {
             varDsc->lvArgReg = theFixedRetBuffReg();
@@ -564,9 +558,6 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo)
 
         CorInfoTypeWithMod corInfoType = info.compCompHnd->getArgType(&info.compMethodInfo->args, argLst, &typeHnd);
         varDsc->lvIsParam              = 1;
-#if ASSERTION_PROP
-        varDsc->lvSingleDef = 1;
-#endif
 
         lvaInitVarDsc(varDsc, varDscInfo->varNum, strip(corInfoType), typeHnd, argLst, &info.compMethodInfo->args);
 
@@ -1046,11 +1037,7 @@ void Compiler::lvaInitGenericsCtxt(InitVarDscInfo* varDscInfo)
 
         LclVarDsc* varDsc = varDscInfo->varDsc;
         varDsc->lvIsParam = 1;
-#if ASSERTION_PROP
-        varDsc->lvSingleDef = 1;
-#endif
-
-        varDsc->lvType = TYP_I_IMPL;
+        varDsc->lvType    = TYP_I_IMPL;
 
         if (varDscInfo->canEnreg(TYP_I_IMPL))
         {
@@ -1112,10 +1099,6 @@ void Compiler::lvaInitVarArgsHandle(InitVarDscInfo* varDscInfo)
         // that other problems are fixed.
         lvaSetVarAddrExposed(varDscInfo->varNum);
 
-#if ASSERTION_PROP
-        varDsc->lvSingleDef = 1;
-#endif
-
         if (varDscInfo->canEnreg(TYP_I_IMPL))
         {
             /* Another register argument */
@@ -2706,54 +2689,60 @@ void Compiler::lvaUpdateClass(unsigned varNum, CORINFO_CLASS_HANDLE clsHnd, bool
     // We should already have a class
     assert(varDsc->lvClassHnd != nullptr);
 
-#if defined(DEBUG)
+    // We should only be updating classes for single-def locals.
+    assert(varDsc->lvSingleDef);
 
-    // In general we only expect one update per local var. However if
-    // a block is re-imported and that block has the only STLOC for
-    // the var, we may see multiple updates. All subsequent updates
-    // should agree on the type, since reimportation is triggered by
-    // type mismatches for things other than ref types.
-    if (varDsc->lvClassInfoUpdated)
-    {
-        assert(varDsc->lvClassHnd == clsHnd);
-        assert(varDsc->lvClassIsExact == isExact);
-    }
+    // Now see if we should update.
+    //
+    // New information may not always be "better" so do some
+    // simple analysis to decide if the update is worthwhile.
+    const bool isNewClass   = (clsHnd != varDsc->lvClassHnd);
+    bool       shouldUpdate = false;
 
-    // This counts as an update, even if nothing changes.
-    varDsc->lvClassInfoUpdated = true;
+    // Are we attempting to update the class? Only check this when we have
+    // an new type and the existing class is inexact... we should not be
+    // updating exact classes.
+    if (!varDsc->lvClassIsExact && isNewClass)
+    {
+        // Todo: improve this analysis by adding a new jit interface method
+        DWORD newAttrs = info.compCompHnd->getClassAttribs(clsHnd);
+        DWORD oldAttrs = info.compCompHnd->getClassAttribs(varDsc->lvClassHnd);
 
-#endif // defined(DEBUG)
+        // Avoid funny things with __Canon by only merging if both shared or both unshared
+        if ((newAttrs & CORINFO_FLG_SHAREDINST) == (oldAttrs & CORINFO_FLG_SHAREDINST))
+        {
+            // If we merge types and we get back the old class, the new class is more
+            // specific and we should update to it.
+            CORINFO_CLASS_HANDLE mergeClass = info.compCompHnd->mergeClasses(clsHnd, varDsc->lvClassHnd);
 
-    // If previous type was exact, there is nothing to update.  Would
-    // like to verify new type is compatible but can't do this yet.
-    if (varDsc->lvClassIsExact)
-    {
-        return;
+            if (mergeClass == varDsc->lvClassHnd)
+            {
+                shouldUpdate = true;
+            }
+        }
+        else if ((newAttrs & CORINFO_FLG_SHAREDINST) == 0)
+        {
+            // Update if we go from shared to unshared
+            shouldUpdate = true;
+        }
     }
-
-    // Are we updating the type?
-    if (varDsc->lvClassHnd != clsHnd)
+    // Else are we attempting to update exactness?
+    else if (isExact && !varDsc->lvClassIsExact && !isNewClass)
     {
-        JITDUMP("\nlvaUpdateClass: Updating class for V%02i from (%p) %s to (%p) %s %s\n", varNum,
-                dspPtr(varDsc->lvClassHnd), info.compCompHnd->getClassName(varDsc->lvClassHnd), dspPtr(clsHnd),
-                info.compCompHnd->getClassName(clsHnd), isExact ? " [exact]" : "");
-
-        varDsc->lvClassHnd     = clsHnd;
-        varDsc->lvClassIsExact = isExact;
-        return;
+        shouldUpdate = true;
     }
 
-    // Class info matched. Are we updating exactness?
-    if (isExact)
-    {
-        JITDUMP("\nlvaUpdateClass: Updating class for V%02i (%p) %s to be exact\n", varNum, dspPtr(varDsc->lvClassHnd),
-                info.compCompHnd->getClassName(varDsc->lvClassHnd));
+    JITDUMP("\nlvaUpdateClass:%s Updating class for V%02u from (%p) %s%s to (%p) %s%s\n", varNum,
+            shouldUpdate ? "" : " NOT", dspPtr(varDsc->lvClassHnd), info.compCompHnd->getClassName(varDsc->lvClassHnd),
+            varDsc->lvClassIsExact ? " [exact]" : "", dspPtr(clsHnd), info.compCompHnd->getClassName(clsHnd),
+            isExact ? " [exact]" : "");
 
+    if (shouldUpdate)
+    {
+        varDsc->lvClassHnd     = clsHnd;
         varDsc->lvClassIsExact = isExact;
-        return;
     }
 
-    // Else we have the same handle and (in)exactness as before. Do nothing.
     return;
 }
 
@@ -4155,6 +4144,10 @@ void Compiler::lvaComputeRefCounts(bool isRecompute, bool setSlotNumbers)
         {
             varDsc->lvSlotNum = lclNum;
         }
+
+        // Set initial value for lvSingleDef for explicit and implicit
+        // argument locals as they are "defined" on entry.
+        varDsc->lvSingleDef = varDsc->lvIsParam;
     }
 
     JITDUMP("\n*** lvaComputeRefCounts -- explicit counts ***\n");