Skip to content

Commit 04bcbec

Browse files
committed
NOP
1 parent 643acd2 commit 04bcbec

File tree

10 files changed

+169
-21
lines changed

10 files changed

+169
-21
lines changed

src/coreclr/jit/codegencommon.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -890,8 +890,8 @@ void CodeGen::genLogLabel(BasicBlock* bb)
890890
void CodeGen::genDefineTempLabel(BasicBlock* label)
891891
{
892892
genLogLabel(label);
893-
label->bbEmitCookie = GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur,
894-
gcInfo.gcRegByrefSetCur DEBUG_ARG(label));
893+
label->bbEmitCookie =
894+
GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur, gcInfo.gcRegByrefSetCur);
895895
}
896896

897897
// genDefineInlineTempLabel: Define an inline label that does not affect the GC

src/coreclr/jit/codegenlinear.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ void CodeGen::genCodeForBBlist()
348348
// Mark a label and update the current set of live GC refs
349349

350350
block->bbEmitCookie = GetEmitter()->emitAddLabel(gcInfo.gcVarPtrSetCur, gcInfo.gcRegGCrefSetCur,
351-
gcInfo.gcRegByrefSetCur DEBUG_ARG(block));
351+
gcInfo.gcRegByrefSetCur, block->Prev());
352352
}
353353

354354
if (block->IsFirstColdBlock(compiler))

src/coreclr/jit/emit.cpp

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2217,6 +2217,10 @@ void emitter::emitCreatePlaceholderIG(insGroupPlaceholderType igType,
22172217
emitCurIG->igFlags &= ~IGF_PROPAGATE_MASK;
22182218
}
22192219

2220+
// since we have emitted a placeholder, the last ins is not longer the last.
2221+
emitLastIns = nullptr;
2222+
emitLastInsIG = nullptr;
2223+
22202224
#ifdef DEBUG
22212225
if (emitComp->verbose)
22222226
{
@@ -2873,10 +2877,36 @@ bool emitter::emitNoGChelper(CORINFO_METHOD_HANDLE methHnd)
28732877
* Mark the current spot as having a label.
28742878
*/
28752879

2876-
void* emitter::emitAddLabel(VARSET_VALARG_TP GCvars,
2877-
regMaskTP gcrefRegs,
2878-
regMaskTP byrefRegs DEBUG_ARG(BasicBlock* block))
2880+
void* emitter::emitAddLabel(VARSET_VALARG_TP GCvars, regMaskTP gcrefRegs, regMaskTP byrefRegs, BasicBlock* prevBlock)
28792881
{
2882+
if (prevBlock != NULL && emitLastInsIsCallWithGC())
2883+
{
2884+
// We have just emitted a call that can do GC and conservatively recorded what is alive after the call.
2885+
// Now we see that the next instruction may be reachable by a branch with a different liveness.
2886+
// We want to maintain the invariant that the GC info at IP after a GC-capable call is the same
2887+
// regardless how it is reached.
2888+
// One way to fix this is to fish out the call instruction and patch its GC info, but we must be
2889+
// certain that the current IP is indeed reachable after the call.
2890+
// Another way it to add an instruction (NOP or BRK) after the call.
2891+
if (emitThisGCrefRegs != gcrefRegs || emitThisByrefRegs != byrefRegs ||
2892+
!VarSetOps::Equal(emitComp, emitThisGCrefVars, GCvars))
2893+
{
2894+
if (prevBlock->KindIs(BBJ_THROW))
2895+
{
2896+
emitIns(INS_BREAKPOINT);
2897+
}
2898+
else
2899+
{
2900+
// other block kinds should emit something at the end that is not a call.
2901+
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);
2906+
}
2907+
}
2908+
}
2909+
28802910
/* Create a new IG if the current one is non-empty */
28812911

28822912
if (emitCurIGnonEmpty())
@@ -3637,6 +3667,7 @@ emitter::instrDesc* emitter::emitNewInstrCallInd(int argCnt,
36373667

36383668
/* Make sure we didn't waste space unexpectedly */
36393669
assert(!id->idIsLargeCns());
3670+
id->idSetIsCall();
36403671

36413672
#ifdef TARGET_XARCH
36423673
/* Store the displacement and make sure the value fit */
@@ -3716,6 +3747,7 @@ emitter::instrDesc* emitter::emitNewInstrCallDir(int argCnt,
37163747

37173748
/* Make sure we didn't waste space unexpectedly */
37183749
assert(!id->idIsLargeCns());
3750+
id->idSetIsCall();
37193751

37203752
/* Save the live GC registers in the unused register fields */
37213753
assert((gcrefRegs & RBM_CALLEE_TRASH) == 0);
@@ -8725,6 +8757,16 @@ void emitter::emitUpdateLiveGCvars(VARSET_VALARG_TP vars, BYTE* addr)
87258757
emitThisGCrefVset = true;
87268758
}
87278759

8760+
/*****************************************************************************
8761+
*
8762+
* Last emitted instruction is a call and it is not a NoGC call.
8763+
*/
8764+
8765+
bool emitter::emitLastInsIsCallWithGC()
8766+
{
8767+
return emitLastIns != nullptr && emitLastIns->idIsCall() && !emitLastIns->idIsNoGC();
8768+
}
8769+
87288770
/*****************************************************************************
87298771
*
87308772
* Record a call location for GC purposes (we know that this is a method that

src/coreclr/jit/emit.h

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -762,10 +762,10 @@ class emitter
762762
// loongarch64: 28 bits
763763
// risc-v: 28 bits
764764

765-
unsigned _idSmallDsc : 1; // is this a "small" descriptor?
766-
unsigned _idLargeCns : 1; // does a large constant follow?
767-
unsigned _idLargeDsp : 1; // does a large displacement follow?
768-
unsigned _idLargeCall : 1; // large call descriptor used
765+
unsigned _idSmallDsc : 1; // is this a "small" descriptor?
766+
unsigned _idLargeCns : 1; // does a large constant follow? (or if large call descriptor used)
767+
unsigned _idLargeDsp : 1; // does a large displacement follow?
768+
unsigned _idCall : 1; // this is a call
769769

770770
// We have several pieces of information we need to encode but which are only applicable
771771
// to a subset of instrDescs. To accommodate that, we define a several _idCustom# bitfields
@@ -1565,7 +1565,7 @@ class emitter
15651565

15661566
bool idIsLargeCns() const
15671567
{
1568-
return _idLargeCns != 0;
1568+
return _idLargeCns != 0 && !idIsCall();
15691569
}
15701570
void idSetIsLargeCns()
15711571
{
@@ -1585,13 +1585,23 @@ class emitter
15851585
_idLargeDsp = 0;
15861586
}
15871587

1588+
bool idIsCall() const
1589+
{
1590+
return _idCall != 0;
1591+
}
1592+
void idSetIsCall()
1593+
{
1594+
_idCall = 1;
1595+
}
1596+
15881597
bool idIsLargeCall() const
15891598
{
1590-
return _idLargeCall != 0;
1599+
return idIsCall() && _idLargeCns == 1;
15911600
}
15921601
void idSetIsLargeCall()
15931602
{
1594-
_idLargeCall = 1;
1603+
idSetIsCall();
1604+
_idLargeCns = 1;
15951605
}
15961606

15971607
bool idIsBound() const
@@ -2857,9 +2867,11 @@ class emitter
28572867
// Mark this instruction group as having a label; return the new instruction group.
28582868
// Sets the emitter's record of the currently live GC variables
28592869
// and registers.
2860-
void* emitAddLabel(VARSET_VALARG_TP GCvars,
2861-
regMaskTP gcrefRegs,
2862-
regMaskTP byrefRegs DEBUG_ARG(BasicBlock* block = nullptr));
2870+
// prevBlock is passed when starting a new block.
2871+
void* emitAddLabel(VARSET_VALARG_TP GCvars,
2872+
regMaskTP gcrefRegs,
2873+
regMaskTP byrefRegs,
2874+
BasicBlock* prevBlock = nullptr);
28632875

28642876
// Same as above, except the label is added and is conceptually "inline" in
28652877
// the current block. Thus it extends the previous block and the emitter
@@ -3190,6 +3202,8 @@ class emitter
31903202

31913203
void emitRecordGCcall(BYTE* codePos, unsigned char callInstrSize);
31923204

3205+
bool emitLastInsIsCallWithGC();
3206+
31933207
// Helpers for the above
31943208

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

src/coreclr/jit/emitarm.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4754,6 +4754,16 @@ void emitter::emitIns_Call(EmitCallType callType,
47544754

47554755
/* Update the emitter's live GC ref sets */
47564756

4757+
// If the method returns a GC ref, mark R0 appropriately
4758+
if (retSize == EA_GCREF)
4759+
{
4760+
gcrefRegs |= RBM_R0;
4761+
}
4762+
else if (retSize == EA_BYREF)
4763+
{
4764+
byrefRegs |= RBM_R0;
4765+
}
4766+
47574767
VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars);
47584768
emitThisGCrefRegs = gcrefRegs;
47594769
emitThisByrefRegs = byrefRegs;

src/coreclr/jit/emitarm64.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9020,6 +9020,26 @@ void emitter::emitIns_Call(EmitCallType callType,
90209020

90219021
/* Update the emitter's live GC ref sets */
90229022

9023+
// If the method returns a GC ref, mark RBM_INTRET appropriately
9024+
if (retSize == EA_GCREF)
9025+
{
9026+
gcrefRegs |= RBM_INTRET;
9027+
}
9028+
else if (retSize == EA_BYREF)
9029+
{
9030+
byrefRegs |= RBM_INTRET;
9031+
}
9032+
9033+
// If is a multi-register return method is called, mark RBM_INTRET_1 appropriately
9034+
if (secondRetSize == EA_GCREF)
9035+
{
9036+
gcrefRegs |= RBM_INTRET_1;
9037+
}
9038+
else if (secondRetSize == EA_BYREF)
9039+
{
9040+
byrefRegs |= RBM_INTRET_1;
9041+
}
9042+
90239043
VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars);
90249044
emitThisGCrefRegs = gcrefRegs;
90259045
emitThisByrefRegs = byrefRegs;

src/coreclr/jit/emitloongarch64.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2464,6 +2464,26 @@ void emitter::emitIns_Call(EmitCallType callType,
24642464

24652465
/* Update the emitter's live GC ref sets */
24662466

2467+
// If the method returns a GC ref, mark RBM_INTRET appropriately
2468+
if (retSize == EA_GCREF)
2469+
{
2470+
gcrefRegs |= RBM_INTRET;
2471+
}
2472+
else if (retSize == EA_BYREF)
2473+
{
2474+
byrefRegs |= RBM_INTRET;
2475+
}
2476+
2477+
// If is a multi-register return method is called, mark RBM_INTRET_1 appropriately
2478+
if (secondRetSize == EA_GCREF)
2479+
{
2480+
gcrefRegs |= RBM_INTRET_1;
2481+
}
2482+
else if (secondRetSize == EA_BYREF)
2483+
{
2484+
byrefRegs |= RBM_INTRET_1;
2485+
}
2486+
24672487
VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars);
24682488
emitThisGCrefRegs = gcrefRegs;
24692489
emitThisByrefRegs = byrefRegs;

src/coreclr/jit/emitriscv64.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,6 +1373,26 @@ void emitter::emitIns_Call(EmitCallType callType,
13731373

13741374
/* Update the emitter's live GC ref sets */
13751375

1376+
// If the method returns a GC ref, mark RBM_INTRET appropriately
1377+
if (retSize == EA_GCREF)
1378+
{
1379+
gcrefRegs |= RBM_INTRET;
1380+
}
1381+
else if (retSize == EA_BYREF)
1382+
{
1383+
byrefRegs |= RBM_INTRET;
1384+
}
1385+
1386+
// If is a multi-register return method is called, mark RBM_INTRET_1 appropriately
1387+
if (secondRetSize == EA_GCREF)
1388+
{
1389+
gcrefRegs |= RBM_INTRET_1;
1390+
}
1391+
else if (secondRetSize == EA_BYREF)
1392+
{
1393+
byrefRegs |= RBM_INTRET_1;
1394+
}
1395+
13761396
VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars);
13771397
emitThisGCrefRegs = gcrefRegs;
13781398
emitThisByrefRegs = byrefRegs;

src/coreclr/jit/emitxarch.cpp

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9578,6 +9578,28 @@ void emitter::emitIns_Call(EmitCallType callType,
95789578

95799579
/* Update the emitter's live GC ref sets */
95809580

9581+
// If the method returns a GC ref, mark EAX appropriately
9582+
if (retSize == EA_GCREF)
9583+
{
9584+
gcrefRegs |= RBM_EAX;
9585+
}
9586+
else if (retSize == EA_BYREF)
9587+
{
9588+
byrefRegs |= RBM_EAX;
9589+
}
9590+
9591+
#ifdef UNIX_AMD64_ABI
9592+
// If is a multi-register return method is called, mark RDX appropriately (for System V AMD64).
9593+
if (secondRetSize == EA_GCREF)
9594+
{
9595+
gcrefRegs |= RBM_RDX;
9596+
}
9597+
else if (secondRetSize == EA_BYREF)
9598+
{
9599+
byrefRegs |= RBM_RDX;
9600+
}
9601+
#endif // UNIX_AMD64_ABI
9602+
95819603
VarSetOps::Assign(emitComp, emitThisGCrefVars, ptrVars);
95829604
emitThisGCrefRegs = gcrefRegs;
95839605
emitThisByrefRegs = byrefRegs;
@@ -16546,9 +16568,9 @@ size_t emitter::emitOutputInstr(insGroup* ig, instrDesc* id, BYTE** dp)
1654616568
#ifdef TARGET_X86
1654716569
dst += emitOutputWord(dst, code | 0x0500);
1654816570
#else // TARGET_AMD64
16549-
// Amd64: addr fits within 32-bits and can be encoded as a displacement relative to zero.
16550-
// This addr mode should never be used while generating relocatable ngen code nor if
16551-
// the addr can be encoded as pc-relative address.
16571+
// Amd64: addr fits within 32-bits and can be encoded as a displacement relative to zero.
16572+
// This addr mode should never be used while generating relocatable ngen code nor if
16573+
// the addr can be encoded as pc-relative address.
1655216574
noway_assert(!emitComp->opts.compReloc);
1655316575
noway_assert(codeGen->genAddrRelocTypeHint((size_t)addr) != IMAGE_REL_BASED_REL32);
1655416576
noway_assert(static_cast<int>(reinterpret_cast<intptr_t>(addr)) == (ssize_t)addr);

src/coreclr/jit/jitgcinfo.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ class GCInfo
183183
}
184184

185185
unsigned short rpdIsThis : 1; // is it the 'this' pointer
186-
unsigned short rpdCall : 1; // is this a true call site?
187-
unsigned short : 1; // Padding bit, so next two start on a byte boundary
186+
unsigned short rpdCall : 1; // is this a true call site?
187+
unsigned short : 1; // Padding bit, so next two start on a byte boundary
188188
unsigned short rpdCallGCrefRegs : CNT_CALL_GC_REGS; // Callee-saved and return registers containing GC pointers.
189189
unsigned short rpdCallByrefRegs : CNT_CALL_GC_REGS; // Callee-saved and return registers containing byrefs.
190190

0 commit comments

Comments
 (0)