Skip to content

Commit

Permalink
Do byref liveness updates for same register GPR moves (dotnet#53684)
Browse files Browse the repository at this point in the history 
* Do byref liveness updates for same register GPR moves on x86/x64

* Change where emitLastEmittedIns is tracked

* Ensure emitLastEmittedIns isn't tracked across instruction groups
  • Loading branch information
@tannergooding
tannergooding authored Jun 9, 2021
1 parent 36a7e76 commit 7df92fd
Show file tree
Hide file tree
Showing 8 changed files with 355 additions and 149 deletions.
86 changes: 79 additions & 7 deletions src/coreclr/jit/emit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1138,7 +1161,8 @@ void emitter::emitBegFN(bool hasFramePtr

emitPrologIG = emitIGlist = emitIGlast = emitCurIG = ig = emitAllocIG();

emitLastIns = nullptr;
emitLastIns = nullptr;
emitLastEmittedIns = nullptr;

ig->igNext = nullptr;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
32 changes: 24 additions & 8 deletions src/coreclr/jit/emit.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -920,7 +928,7 @@ class emitter
break;
}

return size;
return (idIns() != INS_mov_eliminated) ? size : 0;
}

#elif defined(TARGET_ARM)
Expand All @@ -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
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1889,6 +1901,7 @@ class emitter
}

instrDesc* emitLastIns;
instrDesc* emitLastEmittedIns;

#ifdef DEBUG
void emitCheckIGoffsets();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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];

Expand Down
40 changes: 36 additions & 4 deletions src/coreclr/jit/emitarm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 7df92fd

Please sign in to comment.