Do byref liveness updates for same register GPR moves

src/coreclr/jit/emit.cpp

@@ -417,6 +417,8 @@ void emitterStats(FILE* fout)
 #endif // TARGET_ARM
         fprintf(fout, "Total instrDescAlign:  %8u (%5.2f%%)\n", emitter::emitTotalDescAlignCnt,
                 100.0 * emitter::emitTotalDescAlignCnt / emitter::emitTotalInsCnt);
+        fprintf(fout, "Total instrDescZeroSz: %8u (%5.2f%%)\n", emitter::emitTotalDescZeroSzCnt,
+                100.0 * emitter::emitTotalDescZeroSzCnt / emitter::emitTotalInsCnt);
 
         fprintf(fout, "\n");
     }
@@ -665,6 +667,10 @@ void emitter::emitGenIG(insGroup* ig)
     emitCurIGinsCnt = 0;
     emitCurIGsize   = 0;
 
+#if defined(DEBUG)
+    emitCurIGZeroSzCnt = 0;
+#endif // DEBUG
+
     assert(emitCurIGjmpList == nullptr);
 
 #if FEATURE_LOOP_ALIGN
@@ -793,14 +799,21 @@ insGroup* emitter::emitSavIG(bool emitAdd)
     emitCurCodeOffset += emitCurIGsize;
     assert(IsCodeAligned(emitCurCodeOffset));
 
+#if defined(DEBUG) || EMITTER_STATS
+    noway_assert((BYTE)emitCurIGZeroSzCnt == emitCurIGZeroSzCnt);
+    ig->igZeroSzCnt = (BYTE)emitCurIGZeroSzCnt;
+#endif // DEBUG || EMITTER_STATS
+
 #if EMITTER_STATS
     emitTotalIGicnt += emitCurIGinsCnt;
+    emitTotalIGZeroSzCnt += emitCurIGZeroSzCnt;
     emitTotalIGsize += sz;
     emitSizeMethod += sz;
 
     if (emitIGisInProlog(ig))
     {
         emitCurPrologInsCnt += emitCurIGinsCnt;
+        emitCurPrologZeroSzCnt += emitCurIGZeroSzCnt;
         emitCurPrologIGSize += sz;
 
         // Keep track of the maximums.
@@ -993,6 +1006,16 @@ insGroup* emitter::emitSavIG(bool emitAdd)
         assert(emitCurIGfreeBase <= (BYTE*)emitLastIns);
         assert((BYTE*)emitLastIns < emitCurIGfreeBase + sz);
         emitLastIns = (instrDesc*)((BYTE*)id + ((BYTE*)emitLastIns - (BYTE*)emitCurIGfreeBase));
+
+        if (emitLastEmittedIns != nullptr)
+        {
+            // Unlike with emitLastIns, we might be null if the group only contains
+            // elided instructions, in which case we'll only update in that scenario
+
+            assert(emitCurIGfreeBase <= (BYTE*)emitLastEmittedIns);
+            assert((BYTE*)emitLastEmittedIns < emitCurIGfreeBase + sz);
+            emitLastEmittedIns = (instrDesc*)((BYTE*)id + ((BYTE*)emitLastEmittedIns - (BYTE*)emitCurIGfreeBase));
+        }
     }
 
     // Reset the buffer free pointers
@@ -1138,7 +1161,8 @@ void emitter::emitBegFN(bool hasFramePtr
 
     emitPrologIG = emitIGlist = emitIGlast = emitCurIG = ig = emitAllocIG();
 
-    emitLastIns = nullptr;
+    emitLastIns        = nullptr;
+    emitLastEmittedIns = nullptr;
 
     ig->igNext = nullptr;
 
@@ -1197,6 +1221,15 @@ int emitter::instrDesc::idAddrUnion::iiaGetJitDataOffset() const
 //
 float emitter::insEvaluateExecutionCost(instrDesc* id)
 {
+    if (id->idIns() == INS_mov_eliminated)
+    {
+        // Elideable moves are specified to have a zero size, but are carried
+        // in emit so we can still do the relevant byref liveness update
+
+        assert(id->idGCref() == GCT_BYREF);
+        return 0;
+    }
+
     insExecutionCharacteristics result        = getInsExecutionCharacteristics(id);
     float                       throughput    = result.insThroughput;
     float                       latency       = result.insLatency;
@@ -1296,13 +1329,49 @@ void emitter::dispIns(instrDesc* id)
     assert(id->idDebugOnlyInfo()->idSize == sz);
 #endif // DEBUG
 
+#if defined(DEBUG) || EMITTER_STATS
+    if (id->idCodeSize() == 0)
+    {
+        emitCurIGZeroSzCnt++;
+
+#if EMITTER_STATS
+        emitTotalDescZeroSzCnt++
+#endif // EMITTER_STATS
+    }
+#endif // DEBUG || EMITTER_STATS
+
 #if EMITTER_STATS
     emitIFcounts[id->idInsFmt()]++;
-#endif
+#endif // EMITTER_STATS
 }
 
 void emitter::appendToCurIG(instrDesc* id)
 {
+    assert(id == emitLastIns);
+
+    if (emitLastIns->idIns() != INS_mov_eliminated)
+    {
+        // emitAllocAnyInstr sets emitLastIns and id
+        // to be the same. However, for the purposes
+        // of looking back we only want to consider
+        // certain "non-zero" size instructions and
+        // so we'll update the last emitted instruction
+        // when appending to the current IG.
+
+        emitLastEmittedIns = emitLastIns;
+    }
+    else if (emitCurIGsize == 0)
+    {
+        // If we are part of a new instruction group
+        // then we need to null out the last instruction
+        // so that we aren't incorrectly tracking across
+        // block boundaries.
+
+        // TOOD-CQ: We should also be able to track across
+        // extended instruction groups to allow more opts
+
+        emitLastEmittedIns = nullptr;
+    }
     emitCurIGsize += id->idCodeSize();
 }
 
@@ -1416,7 +1485,7 @@ void* emitter::emitAllocAnyInstr(size_t sz, emitAttr opsz)
 #error "Undefined target for pseudorandom NOP insertion"
 #endif
 
-            emitCurIGsize += nopSize;
+            appendToCurIG(emitCurIGsize);
             emitNextNop = emitNextRandomNop();
         }
         else
@@ -4252,15 +4321,15 @@ void emitter::emitJumpDistBind()
 #ifdef TARGET_XARCH
         /* Done if this is not a variable-sized jump */
 
-        if ((jmp->idIns() == INS_push) || (jmp->idIns() == INS_mov) || (jmp->idIns() == INS_call) ||
-            (jmp->idIns() == INS_push_hide))
+        if ((jmp->idIns() == INS_push) || (jmp->idIns() == INS_mov) || (jmp->idIns() == INS_mov_eliminated) ||
+            (jmp->idIns() == INS_call) || (jmp->idIns() == INS_push_hide))
         {
             continue;
         }
 #endif
 #ifdef TARGET_ARM
-        if ((jmp->idIns() == INS_push) || (jmp->idIns() == INS_mov) || (jmp->idIns() == INS_movt) ||
-            (jmp->idIns() == INS_movw))
+        if ((jmp->idIns() == INS_push) || (jmp->idIns() == INS_mov) || (jmp->idIns() == INS_mov_eliminated) ||
+            (jmp->idIns() == INS_movt) || (jmp->idIns() == INS_movw))
         {
             continue;
         }
@@ -5974,6 +6043,9 @@ unsigned emitter::emitEndCodeGen(Compiler* comp,
             printf("\t\t\t\t\t\t;; bbWeight=%s PerfScore %.2f", refCntWtd2str(ig->igWeight), ig->igPerfScore);
         }
         *instrCount += ig->igInsCnt;
+
+        // We don't want to include zero size instructions in the count, as they aren't impactful
+        *instrCount -= ig->igZeroSzCnt;
 #endif // DEBUG
 
         emitCurIG = nullptr;

src/coreclr/jit/emit.h

@@ -299,7 +299,11 @@ struct insGroup
     unsigned igStkLvl; // stack level on entry
 #endif
     regMaskSmall  igGCregs; // set of registers with live GC refs
-    unsigned char igInsCnt; // # of instructions  in this group
+    unsigned char igInsCnt; // # of instructions in this group
+
+#if defined(DEBUG)
+    unsigned char igZeroSzCnt; // # of zero size instructions in this group
+#endif                         // DEBUG
 
 #else // REGMASK_BITS
 
@@ -314,7 +318,11 @@ struct insGroup
     unsigned igStkLvl; // stack level on entry
 #endif
 
-    unsigned char igInsCnt; // # of instructions  in this group
+    unsigned char igInsCnt;    // # of instructions in this group
+
+#if defined(DEBUG)
+    unsigned char igZeroSzCnt; // # of zero size instructions in this group
+#endif // DEBUG
 
 #endif // REGMASK_BITS
 
@@ -920,7 +928,7 @@ class emitter
                     break;
             }
 
-            return size;
+            return (idIns() != INS_mov_eliminated) ? size : 0;
         }
 
 #elif defined(TARGET_ARM)
@@ -932,7 +940,7 @@ class emitter
         unsigned idCodeSize() const
         {
             unsigned result = (_idInsSize == ISZ_16BIT) ? 2 : (_idInsSize == ISZ_32BIT) ? 4 : 6;
-            return result;
+            return (idIns() != INS_mov_eliminated) ? result : 0;
         }
         insSize idInsSize() const
         {
@@ -1793,6 +1801,10 @@ class emitter
     UNATIVE_OFFSET emitCurCodeOffset; // current code offset within group
     UNATIVE_OFFSET emitTotalCodeSize; // bytes of code in entire method
 
+#if defined(DEBUG) || EMITTER_STATS
+    unsigned emitCurIGZeroSzCnt; // # of zero size instr's in buffer
+#endif                           // DEBUG || EMITTER_STATS
+
     insGroup* emitFirstColdIG; // first cold instruction group
 
     void emitSetFirstColdIGCookie(void* bbEmitCookie)
@@ -1889,6 +1901,7 @@ class emitter
     }
 
     instrDesc* emitLastIns;
+    instrDesc* emitLastEmittedIns;
 
 #ifdef DEBUG
     void emitCheckIGoffsets();
@@ -2317,14 +2330,16 @@ class emitter
 
     static unsigned emitTotalInsCnt;
 
-    static unsigned emitCurPrologInsCnt; // current number of prolog instrDescs
-    static size_t   emitCurPrologIGSize; // current size of prolog instrDescs
-    static unsigned emitMaxPrologInsCnt; // maximum number of prolog instrDescs
-    static size_t   emitMaxPrologIGSize; // maximum size of prolog instrDescs
+    static unsigned emitCurPrologInsCnt;    // current number of prolog instrDescs
+    static unsigned emitCurPrologZeroSzCnt; // current number of elided prolog instrDescs
+    static size_t   emitCurPrologIGSize;    // current size of prolog instrDescs
+    static unsigned emitMaxPrologInsCnt;    // maximum number of prolog instrDescs
+    static size_t   emitMaxPrologIGSize;    // maximum size of prolog instrDescs
 
     static unsigned emitTotalIGcnt;   // total number of insGroup allocated
     static unsigned emitTotalPhIGcnt; // total number of insPlaceholderGroupData allocated
     static unsigned emitTotalIGicnt;
+    static unsigned emitTotalIGZeroSzCnt;
     static size_t   emitTotalIGsize;
     static unsigned emitTotalIGmcnt;   // total method count
     static unsigned emitTotalIGExtend; // total number of 'emitExtend' (typically overflow) groups
@@ -2359,6 +2374,7 @@ class emitter
     static unsigned emitSmallCns[SMALL_CNS_TSZ];
     static unsigned emitLargeCnsCnt;
     static unsigned emitTotalDescAlignCnt;
+    static unsigned emitTotalDescZeroSzCnt;
 
     static unsigned emitIFcounts[IF_COUNT];
 

src/coreclr/jit/emitarm.cpp

@@ -1230,6 +1230,7 @@ bool emitter::IsMovInstruction(instruction ins)
     switch (ins)
     {
         case INS_mov:
+        case INS_mov_eliminated:
         case INS_sxtb:
         case INS_sxth:
         case INS_uxtb:
@@ -2065,8 +2066,15 @@ void emitter::emitIns_Mov(instruction ins,
             {
                 if (canSkip && (dstReg == srcReg))
                 {
-                    // These instructions have no side effect and can be skipped
-                    return;
+                    // These instructions might have a side effect in the form of a
+                    // byref liveness update, so preserve them but emit nothing
+
+                    if (!EA_IS_BYREF(attr))
+                    {
+                        return;
+                    }
+
+                    ins = INS_mov_eliminated;
                 }
                 fmt = IF_T1_D0;
                 sf  = INS_FLAGS_NOT_SET;
@@ -5767,6 +5775,18 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
 
     VARSET_TP GCvars(VarSetOps::UninitVal());
 
+    if (ins == INS_mov_eliminated)
+    {
+        // Elideable moves are specified to have a zero size, but are carried
+        // in emit so we can still do the relevant byref liveness update
+
+        assert(id->idGCref() == GCT_BYREF);
+        assert(id->idCodeSize() == 0);
+
+        sz = SMALL_IDSC_SIZE;
+        goto UPDATE_LIVENESS;
+    }
+
     /* What instruction format have we got? */
 
     switch (fmt)
@@ -6587,6 +6607,7 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
             break;
     }
 
+UPDATE_LIVENESS:
     // Determine if any registers now hold GC refs, or whether a register that was overwritten held a GC ref.
     // We assume here that "id->idGCref()" is not GC_NONE only if the instruction described by "id" writes a
     // GC ref to register "id->idReg1()".  (It may, apparently, also not be GC_NONE in other cases, such as
@@ -6703,9 +6724,9 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
     }
 #endif
 
-    /* All instructions are expected to generate code */
+    /* All instructions, except eliminated moves, are expected to generate code */
 
-    assert(*dp != dst);
+    assert((*dp != dst) || (ins == INS_mov_eliminated));
 
     *dp = dst;
 
@@ -7101,6 +7122,17 @@ void emitter::emitDispInsHex(instrDesc* id, BYTE* code, size_t sz)
 void emitter::emitDispInsHelp(
     instrDesc* id, bool isNew, bool doffs, bool asmfm, unsigned offset, BYTE* code, size_t sz, insGroup* ig)
 {
+    if (id->idIns() == INS_mov_eliminated)
+    {
+        // Elideable moves are specified to have a zero size, but are carried
+        // in emit so we can still do the relevant byref liveness update
+
+        assert(id->idGCref() == GCT_BYREF);
+        assert(id->idCodeSize() == 0);
+
+        return;
+    }
+
     if (EMITVERBOSE)
     {
         unsigned idNum = id->idDebugOnlyInfo()->idNum; // Do not remove this!  It is needed for VisualStudio