Skip to content

Commit d7fa143

Browse files
VSadovVSadov
committed
No NOP
1 parent 04bcbec commit d7fa143

File tree

6 files changed

+98
-65
lines changed

6 files changed

+98
-65
lines changed

‎src/coreclr/jit/emit.cpp

+70-5
Original file line numberDiff line numberDiff line change
@@ -2897,12 +2897,77 @@ void* emitter::emitAddLabel(VARSET_VALARG_TP GCvars, regMaskTP gcrefRegs, regMas
28972897
}
28982898
else
28992899
{
2900-
// other block kinds should emit something at the end that is not a call.
2900+
// other block kinds should emit something that is not a call at the end of the block.
29012901
assert(prevBlock->KindIs(BBJ_ALWAYS));
2902-
// CONSIDER: We could patch up the previous call instruction with new GC info instead.
2903-
// But that will need to be coordinated with how the GC info vor variables is used.
2904-
// We currently apply that info to the instruction before the call. It may need to change.
2905-
emitIns(INS_nop);
2902+
2903+
instrDesc* id = emitLastIns;
2904+
regMaskTP callGcrefRegs = gcrefRegs;
2905+
regMaskTP callByrefRegs = byrefRegs;
2906+
2907+
// We may see returns that become alive after the call,
2908+
// but those are tracked via idGCref/idSecondGCref. We do not need to track those in the call.
2909+
if (id->idGCref() == GCT_GCREF)
2910+
{
2911+
callGcrefRegs &= ~RBM_INTRET;
2912+
}
2913+
else if (id->idGCref() == GCT_BYREF)
2914+
{
2915+
callByrefRegs &= ~RBM_INTRET;
2916+
}
2917+
2918+
if (id->idIsLargeCall())
2919+
{
2920+
instrDescCGCA* idCall = (instrDescCGCA*)id;
2921+
#if MULTIREG_HAS_SECOND_GC_RET
2922+
if (idCall->idSecondGCref() == GCT_GCREF)
2923+
{
2924+
callGcrefRegs &= ~RBM_INTRET_1;
2925+
}
2926+
else if (idCall->idSecondGCref() == GCT_BYREF)
2927+
{
2928+
callByrefRegs &= ~RBM_INTRET_1;
2929+
}
2930+
#endif // MULTIREG_HAS_SECOND_GC_RET
2931+
2932+
// new set must be a subset of old one
2933+
if ((idCall->idcGcrefRegs & callGcrefRegs) == callGcrefRegs &&
2934+
(idCall->idcByrefRegs & callByrefRegs) == callByrefRegs &&
2935+
VarSetOps::IsSubset(emitComp, GCvars, idCall->idcGCvars))
2936+
{
2937+
// Update the liveness set.
2938+
VarSetOps::Assign(emitComp, idCall->idcGCvars, GCvars);
2939+
idCall->idcGcrefRegs = callGcrefRegs;
2940+
idCall->idcByrefRegs = callByrefRegs;
2941+
}
2942+
else
2943+
{
2944+
// I have never seen this triggered with large calls,
2945+
// but with small calls below it happens (although rarely)
2946+
assert(!"The live set is expanding!!!!");
2947+
2948+
emitIns(INS_BREAKPOINT);
2949+
}
2950+
}
2951+
else
2952+
{
2953+
assert((callGcrefRegs & RBM_CALLEE_TRASH) == 0);
2954+
2955+
// new set must be a subset of old one
2956+
if ((emitDecodeCallGCregs(id) & callGcrefRegs) == callGcrefRegs && callByrefRegs == RBM_NONE &&
2957+
VarSetOps::IsEmpty(emitComp, GCvars))
2958+
{
2959+
// Update the liveness set.
2960+
emitEncodeCallGCregs(callGcrefRegs, id);
2961+
}
2962+
else
2963+
{
2964+
// The live set is expanding!!!!
2965+
// We branch here with live byref regs, which are not returns, and the call did not record any.
2966+
// Not sure why we see this, but it only can work if the label is unreachable from the call.
2967+
// Perhaps BBJ_ALWAYS somehow ended with a throwing call?
2968+
emitIns(INS_BREAKPOINT);
2969+
}
2970+
}
29062971
}
29072972
}
29082973
}

‎src/coreclr/jit/emitarm.cpp

+2-8
Original file line numberDiff line numberDiff line change
@@ -6524,16 +6524,10 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
65246524

65256525
DONE_CALL:
65266526

6527-
/* We update the GC info before the call as the variables cannot be
6528-
used by the call. Killing variables before the call helps with
6529-
boundary conditions if the call is CORINFO_HELP_THROW - see bug 50029.
6530-
If we ever track aliased variables (which could be used by the
6531-
call), we would have to keep them alive past the call. */
6532-
6533-
emitUpdateLiveGCvars(GCvars, *dp);
6527+
emitUpdateLiveGCvars(GCvars, dst);
65346528

65356529
#ifdef DEBUG
6536-
// Output any delta in GC variable info, corresponding to the before-call GC var updates done above.
6530+
// Output any delta in GC variable info, corresponding to the GC var updates done above.
65376531
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
65386532
{
65396533
emitDispGCVarDelta();

‎src/coreclr/jit/emitarm64.cpp

+4-10
Original file line numberDiff line numberDiff line change
@@ -10695,11 +10695,10 @@ unsigned emitter::emitOutputCall(insGroup* ig, BYTE* dst, instrDesc* id, code_t
1069510695
VarSetOps::AssignNoCopy(emitComp, GCvars, VarSetOps::MakeEmpty(emitComp));
1069610696
}
1069710697

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

1070410703
emitUpdateLiveGCvars(GCvars, dst);
1070510704

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

10714-
// Now output the call instruction and update the 'dst' pointer
10715-
//
10716-
unsigned outputInstrSize = emitOutput_Instr(dst, code);
10717-
dst += outputInstrSize;
10718-
1071910713
// All call instructions are 4-byte in size on ARM64
1072010714
//
1072110715
assert(outputInstrSize == callInstrSize);

‎src/coreclr/jit/emitloongarch64.cpp

+10-16
Original file line numberDiff line numberDiff line change
@@ -2606,22 +2606,6 @@ unsigned emitter::emitOutputCall(insGroup* ig, BYTE* dst, instrDesc* id, code_t
26062606
VarSetOps::AssignNoCopy(emitComp, GCvars, VarSetOps::MakeEmpty(emitComp));
26072607
}
26082608

2609-
/* We update the GC info before the call as the variables cannot be
2610-
used by the call. Killing variables before the call helps with
2611-
boundary conditions if the call is CORINFO_HELP_THROW - see bug 50029.
2612-
If we ever track aliased variables (which could be used by the
2613-
call), we would have to keep them alive past the call. */
2614-
2615-
emitUpdateLiveGCvars(GCvars, dst);
2616-
#ifdef DEBUG
2617-
// NOTEADD:
2618-
// Output any delta in GC variable info, corresponding to the before-call GC var updates done above.
2619-
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
2620-
{
2621-
emitDispGCVarDelta(); // define in emit.cpp
2622-
}
2623-
#endif // DEBUG
2624-
26252609
assert(id->idIns() == INS_jirl);
26262610
if (id->idIsCallRegPtr())
26272611
{ // EC_INDIR_R
@@ -2705,6 +2689,16 @@ unsigned emitter::emitOutputCall(insGroup* ig, BYTE* dst, instrDesc* id, code_t
27052689

27062690
dst += 4;
27072691

2692+
emitUpdateLiveGCvars(GCvars, dst);
2693+
2694+
#ifdef DEBUG
2695+
// Output any delta in GC variable info, corresponding to the GC var updates done above.
2696+
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
2697+
{
2698+
emitDispGCVarDelta(); // define in emit.cpp
2699+
}
2700+
#endif // DEBUG
2701+
27082702
// If the method returns a GC ref, mark INTRET (A0) appropriately.
27092703
if (id->idGCref() == GCT_GCREF)
27102704
{

‎src/coreclr/jit/emitriscv64.cpp

+10-16
Original file line numberDiff line numberDiff line change
@@ -1515,22 +1515,6 @@ unsigned emitter::emitOutputCall(const insGroup* ig, BYTE* dst, instrDesc* id, c
15151515
VarSetOps::AssignNoCopy(emitComp, GCvars, VarSetOps::MakeEmpty(emitComp));
15161516
}
15171517

1518-
/* We update the GC info before the call as the variables cannot be
1519-
used by the call. Killing variables before the call helps with
1520-
boundary conditions if the call is CORINFO_HELP_THROW - see bug 50029.
1521-
If we ever track aliased variables (which could be used by the
1522-
call), we would have to keep them alive past the call. */
1523-
1524-
emitUpdateLiveGCvars(GCvars, dst);
1525-
#ifdef DEBUG
1526-
// NOTEADD:
1527-
// Output any delta in GC variable info, corresponding to the before-call GC var updates done above.
1528-
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
1529-
{
1530-
emitDispGCVarDelta(); // define in emit.cpp
1531-
}
1532-
#endif // DEBUG
1533-
15341518
assert(id->idIns() == INS_jalr);
15351519
if (id->idIsCallRegPtr())
15361520
{ // EC_INDIR_R
@@ -1651,6 +1635,16 @@ unsigned emitter::emitOutputCall(const insGroup* ig, BYTE* dst, instrDesc* id, c
16511635

16521636
dst += 4;
16531637

1638+
emitUpdateLiveGCvars(GCvars, dst);
1639+
1640+
#ifdef DEBUG
1641+
// Output any delta in GC variable info, corresponding to the GC var updates done above.
1642+
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
1643+
{
1644+
emitDispGCVarDelta(); // define in emit.cpp
1645+
}
1646+
#endif // DEBUG
1647+
16541648
// If the method returns a GC ref, mark INTRET (A0) appropriately.
16551649
if (id->idGCref() == GCT_GCREF)
16561650
{

‎src/coreclr/jit/emitxarch.cpp

+2-10
Original file line numberDiff line numberDiff line change
@@ -16639,21 +16639,13 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
1663916639
recCall = false;
1664016640
}
1664116641

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

16651-
// Note the use of address `*dp`, the call instruction address, instead of `dst`, the post-call-instruction
16652-
// address.
16653-
emitUpdateLiveGCvars(GCvars, *dp);
16645+
emitUpdateLiveGCvars(GCvars, dst);
1665416646

1665516647
#ifdef DEBUG
16656-
// Output any delta in GC variable info, corresponding to the before-call GC var updates done above.
16648+
// Output any delta in GC variable info, corresponding to the GC var updates done above.
1665716649
if (EMIT_GC_VERBOSE || emitComp->opts.disasmWithGC)
1665816650
{
1665916651
emitDispGCVarDelta();

0 commit comments

Comments
 (0)