Skip to content

Commit

Permalink
No NOP
Browse files Browse the repository at this point in the history
  • Loading branch information
@VSadov
VSadov committed Apr 11, 2024
1 parent 04bcbec commit 3e2fb84
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 65 deletions.
79 changes: 74 additions & 5 deletions src/coreclr/jit/emit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2897,12 +2897,81 @@ void* emitter::emitAddLabel(VARSET_VALARG_TP GCvars, regMaskTP gcrefRegs, regMas
}
else
{
// other block kinds should emit something at the end that is not a call.
// other block kinds should emit something that is not a call at the end of the block.
assert(prevBlock->KindIs(BBJ_ALWAYS));
// CONSIDER: We could patch up the previous call instruction with new GC info instead.
// But that will need to be coordinated with how the GC info vor variables is used.
// We currently apply that info to the instruction before the call. It may need to change.
emitIns(INS_nop);

instrDesc* id = emitLastIns;
regMaskTP callGcrefRegs = gcrefRegs;
regMaskTP callByrefRegs = byrefRegs;

// We may see returns that become alive after the call,
// but those are tracked via idGCref/idSecondGCref. We do not need to track those in the call.
if (id->idGCref() == GCT_GCREF)
{
callGcrefRegs &= ~RBM_INTRET;
}
else if (id->idGCref() == GCT_BYREF)
{
callByrefRegs &= ~RBM_INTRET;
}

if (id->idIsLargeCall())
{
instrDescCGCA* idCall = (instrDescCGCA*)id;
#if MULTIREG_HAS_SECOND_GC_RET
if (idCall->idSecondGCref() == GCT_GCREF)
{
callGcrefRegs &= ~RBM_INTRET_1;
}
else if (idCall->idSecondGCref() == GCT_BYREF)
{
callByrefRegs &= ~RBM_INTRET_1;
}
#endif // MULTIREG_HAS_SECOND_GC_RET

// new set must be a subset of old one
if ((idCall->idcGcrefRegs & callGcrefRegs) == callGcrefRegs &&
(idCall->idcByrefRegs & callByrefRegs) == callByrefRegs &&
VarSetOps::IsSubset(emitComp, GCvars, idCall->idcGCvars))
{
// Update the liveness set.
VarSetOps::Assign(emitComp, idCall->idcGCvars, GCvars);
idCall->idcGcrefRegs = callGcrefRegs;
idCall->idcByrefRegs = callByrefRegs;
}
else
{
// I have never seen this triggered with large calls,
// but with small calls below it happens (although rarely)
assert(!"The live set is expanding!!!!");

// The live set is expanding!!!!
// We branch here with live byref regs, which are not returns, and the call did not record any.
// Not sure why we see this, but it only can work if the label is unreachable from the call.
// Perhaps BBJ_ALWAYS somehow ended with a throwing call?
emitIns(INS_BREAKPOINT);
}
}
else
{
assert((callGcrefRegs & RBM_CALLEE_TRASH) == 0);

// new set must be a subset of old one
if ((emitDecodeCallGCregs(id) & callGcrefRegs) == callGcrefRegs && callByrefRegs == RBM_NONE &&
VarSetOps::IsEmpty(emitComp, GCvars))
{
// Update the liveness set.
emitEncodeCallGCregs(callGcrefRegs, id);
}
else
{
// The live set is expanding!!!!
// We branch here with live byref regs, which are not returns, and the call did not record any.
// Not sure why we see this, but it only can work if the label is unreachable from the call.
// Perhaps BBJ_ALWAYS somehow ended with a throwing call?
emitIns(INS_BREAKPOINT);
}
}
}
}
}
Expand Down
10 changes: 2 additions & 8 deletions src/coreclr/jit/emitarm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6524,16 +6524,10 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)

DONE_CALL:

/* We update the GC info before the call as the variables cannot be
used by the call. Killing variables before the call helps with
boundary conditions if the call is CORINFO_HELP_THROW - see bug 50029.
If we ever track aliased variables (which could be used by the
call), we would have to keep them alive past the call. */

emitUpdateLiveGCvars(GCvars, *dp);
emitUpdateLiveGCvars(GCvars, dst);

#ifdef DEBUG
// Output any delta in GC variable info, corresponding to the before-call GC var updates done above.
// Output any delta in GC variable info, corresponding to the GC var updates done above.
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
{
emitDispGCVarDelta();
Expand Down
14 changes: 4 additions & 10 deletions src/coreclr/jit/emitarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10695,11 +10695,10 @@ unsigned emitter::emitOutputCall(insGroup* ig, BYTE* dst, instrDesc* id, code_t
VarSetOps::AssignNoCopy(emitComp, GCvars, VarSetOps::MakeEmpty(emitComp));
}

/* We update the GC info before the call as the variables cannot be
used by the call. Killing variables before the call helps with
boundary conditions if the call is CORINFO_HELP_THROW - see bug 50029.
If we ever track aliased variables (which could be used by the
call), we would have to keep them alive past the call. */
// Now output the call instruction and update the 'dst' pointer
//
unsigned outputInstrSize = emitOutput_Instr(dst, code);
dst += outputInstrSize;

emitUpdateLiveGCvars(GCvars, dst);

Expand All @@ -10711,11 +10710,6 @@ unsigned emitter::emitOutputCall(insGroup* ig, BYTE* dst, instrDesc* id, code_t
}
#endif // DEBUG

// Now output the call instruction and update the 'dst' pointer
//
unsigned outputInstrSize = emitOutput_Instr(dst, code);
dst += outputInstrSize;

// All call instructions are 4-byte in size on ARM64
//
assert(outputInstrSize == callInstrSize);
Expand Down
26 changes: 10 additions & 16 deletions src/coreclr/jit/emitloongarch64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2606,22 +2606,6 @@ unsigned emitter::emitOutputCall(insGroup* ig, BYTE* dst, instrDesc* id, code_t
VarSetOps::AssignNoCopy(emitComp, GCvars, VarSetOps::MakeEmpty(emitComp));
}

/* We update the GC info before the call as the variables cannot be
used by the call. Killing variables before the call helps with
boundary conditions if the call is CORINFO_HELP_THROW - see bug 50029.
If we ever track aliased variables (which could be used by the
call), we would have to keep them alive past the call. */

emitUpdateLiveGCvars(GCvars, dst);
#ifdef DEBUG
// NOTEADD:
// Output any delta in GC variable info, corresponding to the before-call GC var updates done above.
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
{
emitDispGCVarDelta(); // define in emit.cpp
}
#endif // DEBUG

assert(id->idIns() == INS_jirl);
if (id->idIsCallRegPtr())
{ // EC_INDIR_R
Expand Down Expand Up @@ -2705,6 +2689,16 @@ unsigned emitter::emitOutputCall(insGroup* ig, BYTE* dst, instrDesc* id, code_t

dst += 4;

emitUpdateLiveGCvars(GCvars, dst);

#ifdef DEBUG
// Output any delta in GC variable info, corresponding to the GC var updates done above.
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
{
emitDispGCVarDelta(); // define in emit.cpp
}
#endif // DEBUG

// If the method returns a GC ref, mark INTRET (A0) appropriately.
if (id->idGCref() == GCT_GCREF)
{
Expand Down
26 changes: 10 additions & 16 deletions src/coreclr/jit/emitriscv64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1515,22 +1515,6 @@ unsigned emitter::emitOutputCall(const insGroup* ig, BYTE* dst, instrDesc* id, c
VarSetOps::AssignNoCopy(emitComp, GCvars, VarSetOps::MakeEmpty(emitComp));
}

/* We update the GC info before the call as the variables cannot be
used by the call. Killing variables before the call helps with
boundary conditions if the call is CORINFO_HELP_THROW - see bug 50029.
If we ever track aliased variables (which could be used by the
call), we would have to keep them alive past the call. */

emitUpdateLiveGCvars(GCvars, dst);
#ifdef DEBUG
// NOTEADD:
// Output any delta in GC variable info, corresponding to the before-call GC var updates done above.
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
{
emitDispGCVarDelta(); // define in emit.cpp
}
#endif // DEBUG

assert(id->idIns() == INS_jalr);
if (id->idIsCallRegPtr())
{ // EC_INDIR_R
Expand Down Expand Up @@ -1651,6 +1635,16 @@ unsigned emitter::emitOutputCall(const insGroup* ig, BYTE* dst, instrDesc* id, c

dst += 4;

emitUpdateLiveGCvars(GCvars, dst);

#ifdef DEBUG
// Output any delta in GC variable info, corresponding to the GC var updates done above.
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
{
emitDispGCVarDelta(); // define in emit.cpp
}
#endif // DEBUG

// If the method returns a GC ref, mark INTRET (A0) appropriately.
if (id->idGCref() == GCT_GCREF)
{
Expand Down
12 changes: 2 additions & 10 deletions src/coreclr/jit/emitxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16639,21 +16639,13 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
recCall = false;
}

/* We update the variable (not register) GC info before the call as the variables cannot be
used by the call. Killing variables before the call helps with
boundary conditions if the call is CORINFO_HELP_THROW - see bug 50029.
If we ever track aliased variables (which could be used by the
call), we would have to keep them alive past the call.
*/
assert(FitsIn<unsigned char>(dst - *dp));
callInstrSize = static_cast<unsigned char>(dst - *dp);

// Note the use of address `*dp`, the call instruction address, instead of `dst`, the post-call-instruction
// address.
emitUpdateLiveGCvars(GCvars, *dp);
emitUpdateLiveGCvars(GCvars, dst);

#ifdef DEBUG
// Output any delta in GC variable info, corresponding to the before-call GC var updates done above.
// Output any delta in GC variable info, corresponding to the GC var updates done above.
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
{
emitDispGCVarDelta();
Expand Down

0 comments on commit 3e2fb84

Please sign in to comment.