Skip to content

Commit 852f8c6

Browse files
committed
NOP
1 parent 45ba695 commit 852f8c6

File tree

3 files changed

+61
-12
lines changed

3 files changed

+61
-12
lines changed

src/coreclr/jit/codegenlinear.cpp

+28-1
Original file line numberDiff line numberDiff line change
@@ -774,19 +774,46 @@ void CodeGen::genCodeForBBlist()
774774
// If this block jumps to the next one, we might be able to skip emitting the jump
775775
if (block->CanRemoveJumpToNext(compiler))
776776
{
777+
removedJmp = true;
777778
#ifdef TARGET_AMD64
778779
if (emitNopBeforeEHRegion)
779780
{
780781
instGen(INS_nop);
782+
break;
781783
}
782784
#endif // TARGET_AMD64
783785

784-
removedJmp = true;
786+
// if liveness is changing and the last block ended on emitting a call that can do GC,
787+
// emit a NOP to ensure that GC info is not changing between
788+
// "call has been made" and "call has returned" states.
789+
if (GetEmitter()->emitLastInsIsCallWithGC())
790+
{
791+
BasicBlock* nextBlock = block->Next();
792+
if (nextBlock && !VarSetOps::Equal(compiler, block->bbLiveOut, nextBlock->bbLiveIn))
793+
{
794+
// sanity check: we can only see a reduction of live variable set.
795+
assert(VarSetOps::IsSubset(compiler, nextBlock->bbLiveIn, block->bbLiveOut));
796+
instGen(INS_nop);
797+
}
798+
}
799+
785800
break;
786801
}
787802
#ifdef TARGET_XARCH
788803
// Do not remove a jump between hot and cold regions.
789804
bool isRemovableJmpCandidate = !compiler->fgInDifferentRegions(block, block->GetTarget());
805+
// if liveness is changing and we've just emitted a GC-capable call, the jump is not removable
806+
// to ensure that GC info is not changing between
807+
// "call has been made" and "call has returned" states.
808+
if (isRemovableJmpCandidate && GetEmitter()->emitLastInsIsCallWithGC())
809+
{
810+
if (!VarSetOps::Equal(compiler, block->bbLiveOut, block->GetTarget()->bbLiveIn))
811+
{
812+
// sanity check: we can only see a reduction of live variable set.
813+
assert(VarSetOps::IsSubset(compiler, block->GetTarget()->bbLiveIn, block->bbLiveOut));
814+
isRemovableJmpCandidate = false;
815+
}
816+
}
790817

791818
inst_JMP(EJ_jmp, block->GetTarget(), isRemovableJmpCandidate);
792819
#else

src/coreclr/jit/emit.cpp

+18-3
Original file line numberDiff line numberDiff line change
@@ -3640,8 +3640,9 @@ emitter::instrDesc* emitter::emitNewInstrCallInd(int argCnt,
36403640
instrDescCGCA* id;
36413641

36423642
id = emitAllocInstrCGCA(retSize);
3643-
3644-
id->idSetIsLargeCall();
3643+
id->idSetIsLargeCns();
3644+
id->idSetIsCall();
3645+
assert(id->idIsLargeCall());
36453646

36463647
VarSetOps::Assign(emitComp, id->idcGCvars, GCvars);
36473648
id->idcGcrefRegs = gcrefRegs;
@@ -3663,6 +3664,7 @@ emitter::instrDesc* emitter::emitNewInstrCallInd(int argCnt,
36633664

36643665
/* Make sure we didn't waste space unexpectedly */
36653666
assert(!id->idIsLargeCns());
3667+
id->idSetIsCall();
36663668

36673669
#ifdef TARGET_XARCH
36683670
/* Store the displacement and make sure the value fit */
@@ -3720,7 +3722,9 @@ emitter::instrDesc* emitter::emitNewInstrCallDir(int argCnt,
37203722

37213723
// printf("Direct call with GC vars / big arg cnt / explicit scope\n");
37223724

3723-
id->idSetIsLargeCall();
3725+
id->idSetIsLargeCns();
3726+
id->idSetIsCall();
3727+
assert(id->idIsLargeCall());
37243728

37253729
VarSetOps::Assign(emitComp, id->idcGCvars, GCvars);
37263730
id->idcGcrefRegs = gcrefRegs;
@@ -3742,6 +3746,7 @@ emitter::instrDesc* emitter::emitNewInstrCallDir(int argCnt,
37423746

37433747
/* Make sure we didn't waste space unexpectedly */
37443748
assert(!id->idIsLargeCns());
3749+
id->idSetIsCall();
37453750

37463751
/* Save the live GC registers in the unused register fields */
37473752
assert((gcrefRegs & RBM_CALLEE_TRASH) == 0);
@@ -8763,6 +8768,16 @@ void emitter::emitUpdateLiveGCvars(VARSET_VALARG_TP vars, BYTE* addr)
87638768
emitThisGCrefVset = true;
87648769
}
87658770

8771+
/*****************************************************************************
8772+
*
8773+
* Last emitted instruction is a call that is not a NoGC call.
8774+
*/
8775+
8776+
bool emitter::emitLastInsIsCallWithGC()
8777+
{
8778+
return emitLastIns != nullptr && emitLastIns->idIsCall() && !emitLastIns->idIsNoGC();
8779+
}
8780+
87668781
/*****************************************************************************
87678782
*
87688783
* Record a call location for GC purposes (we know that this is a method that

src/coreclr/jit/emit.h

+15-8
Original file line numberDiff line numberDiff line change
@@ -763,10 +763,10 @@ class emitter
763763
// loongarch64: 28 bits
764764
// risc-v: 28 bits
765765

766-
unsigned _idSmallDsc : 1; // is this a "small" descriptor?
767-
unsigned _idLargeCns : 1; // does a large constant follow?
768-
unsigned _idLargeDsp : 1; // does a large displacement follow?
769-
unsigned _idLargeCall : 1; // large call descriptor used
766+
unsigned _idSmallDsc : 1; // is this a "small" descriptor?
767+
unsigned _idLargeCns : 1; // does a large constant follow?
768+
unsigned _idLargeDsp : 1; // does a large displacement follow?
769+
unsigned _idCall : 1; // this is a call
770770

771771
// We have several pieces of information we need to encode but which are only applicable
772772
// to a subset of instrDescs. To accommodate that, we define a several _idCustom# bitfields
@@ -1592,13 +1592,18 @@ class emitter
15921592
_idLargeDsp = 0;
15931593
}
15941594

1595-
bool idIsLargeCall() const
1595+
bool idIsCall() const
1596+
{
1597+
return _idCall != 0;
1598+
}
1599+
void idSetIsCall()
15961600
{
1597-
return _idLargeCall != 0;
1601+
_idCall = 1;
15981602
}
1599-
void idSetIsLargeCall()
1603+
1604+
bool idIsLargeCall() const
16001605
{
1601-
_idLargeCall = 1;
1606+
return idIsCall() && idIsLargeCns();
16021607
}
16031608

16041609
bool idIsBound() const
@@ -3196,6 +3201,8 @@ class emitter
31963201

31973202
void emitRecordGCcall(BYTE* codePos, unsigned char callInstrSize);
31983203

3204+
bool emitLastInsIsCallWithGC();
3205+
31993206
// Helpers for the above
32003207

32013208
void emitStackPushLargeStk(BYTE* addr, GCtype gcType, unsigned count = 1);

0 commit comments

Comments
 (0)