Skip to content

Commit 3bda307

Browse files
committed
NOP
1 parent c77dbfb commit 3bda307

7 files changed

+50
-14
lines changed

src/coreclr/jit/codegencommon.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -502,9 +502,9 @@ void CodeGenInterface::genUpdateLife(GenTree* tree)
502502
treeLifeUpdater->UpdateLife(tree);
503503
}
504504

505-
void CodeGenInterface::genUpdateLife(VARSET_VALARG_TP newLife)
505+
bool CodeGenInterface::genUpdateLife(VARSET_VALARG_TP newLife)
506506
{
507-
compiler->compUpdateLife</*ForCodeGen*/ true>(newLife);
507+
return compiler->compUpdateLife</*ForCodeGen*/ true>(newLife);
508508
}
509509

510510
// Return the register mask for the given register variable

src/coreclr/jit/codegeninterface.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class CodeGenInterface
165165
regMaskTP genGetRegMask(GenTree* tree);
166166

167167
void genUpdateLife(GenTree* tree);
168-
void genUpdateLife(VARSET_VALARG_TP newLife);
168+
bool genUpdateLife(VARSET_VALARG_TP newLife);
169169

170170
TreeLifeUpdater<true>* treeLifeUpdater;
171171

src/coreclr/jit/codegenlinear.cpp

+12-1
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,18 @@ void CodeGen::genCodeForBBlist()
194194

195195
// Updating variable liveness after last instruction of previous block was emitted
196196
// and before first of the current block is emitted
197-
genUpdateLife(block->bbLiveIn);
197+
bool livenessChanged = genUpdateLife(block->bbLiveIn);
198+
199+
// if liveness has changed and the last block ended on emitting a call that can do GC,
200+
// emit a nop to ensure that GC info is not changing between
201+
// "call has been made" and "call has returned" states.
202+
if (livenessChanged)
203+
{
204+
if (GetEmitter()->emitLastInsIsCallWithGC())
205+
{
206+
instGen(INS_nop);
207+
}
208+
}
198209

199210
// Even if liveness didn't change, we need to update the registers containing GC references.
200211
// genUpdateLife will update the registers live due to liveness changes. But what about registers that didn't

src/coreclr/jit/compiler.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -8516,7 +8516,7 @@ class Compiler
85168516
// Update the GC's masks, register's masks and reports change on variable's homes given a set of
85178517
// current live variables if changes have happened since "compCurLife".
85188518
template <bool ForCodeGen>
8519-
inline void compUpdateLife(VARSET_VALARG_TP newLife);
8519+
inline bool compUpdateLife(VARSET_VALARG_TP newLife);
85208520

85218521
// Gets a register mask that represent the kill set for a helper call since
85228522
// not all JIT Helper calls follow the standard ABI on the target architecture.

src/coreclr/jit/compiler.hpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -3543,11 +3543,12 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
35433543
// The set of live variables reflects the result of only emitted code, it should not be considering the becoming
35443544
// live/dead of instructions that has not been emitted yet. This is requires by "compChangeLife".
35453545
template <bool ForCodeGen>
3546-
inline void Compiler::compUpdateLife(VARSET_VALARG_TP newLife)
3546+
inline bool Compiler::compUpdateLife(VARSET_VALARG_TP newLife)
35473547
{
35483548
if (!VarSetOps::Equal(this, compCurLife, newLife))
35493549
{
35503550
compChangeLife<ForCodeGen>(newLife);
3551+
return true;
35513552
}
35523553
#ifdef DEBUG
35533554
else
@@ -3560,6 +3561,8 @@ inline void Compiler::compUpdateLife(VARSET_VALARG_TP newLife)
35603561
}
35613562
}
35623563
#endif // DEBUG
3564+
3565+
return false;
35633566
}
35643567

35653568
/*****************************************************************************

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

+12-5
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ class emitter
766766
unsigned _idSmallDsc : 1; // is this a "small" descriptor?
767767
unsigned _idLargeCns : 1; // does a large constant follow?
768768
unsigned _idLargeDsp : 1; // does a large displacement follow?
769-
unsigned _idLargeCall : 1; // large call descriptor used
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)