Skip to content

Commit

Permalink
Allow GC on the first instruction of NoGC region (except in prologs/e…
Browse files Browse the repository at this point in the history 
…pilogs) (#103540)

* x64

* other platforms

* no GC in epilogs

* missing change for arm32

* add asserts

* Addressing PR feedback.

* whitespace formatting
  • Loading branch information
@VSadov
VSadov authored Jun 24, 2024
1 parent d2eca3b commit 9ef950b
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 52 deletions.
10 changes: 5 additions & 5 deletions src/coreclr/jit/codegenarm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
assert(block->KindIs(BBJ_CALLFINALLY));

GetEmitter()->emitIns_J(INS_bl, block->GetTarget());

BasicBlock* nextBlock = block->Next();

if (block->HasFlag(BBF_RETLESS_CALL))
{
GetEmitter()->emitIns_J(INS_bl, block->GetTarget());

if ((nextBlock == nullptr) || !BasicBlock::sameEHRegion(block, nextBlock))
{
instGen(INS_BREAKPOINT);
Expand All @@ -138,12 +138,12 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)

// Because of the way the flowgraph is connected, the liveness info for this one instruction
// after the call is not (can not be) correct in cases where a variable has a last use in the
// handler. So turn off GC reporting for this single instruction.
// handler. So turn off GC reporting once we execute the call and reenable after the jmp/nop
GetEmitter()->emitDisableGC();

BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
GetEmitter()->emitIns_J(INS_bl, block->GetTarget());

// Now go to where the finally funclet needs to return to.
BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
if (nextBlock->NextIs(finallyContinuation) && !compiler->fgInDifferentRegions(nextBlock, finallyContinuation))
{
// Fall-through.
Expand Down
13 changes: 7 additions & 6 deletions src/coreclr/jit/codegenarm64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2146,6 +2146,8 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
assert(block->KindIs(BBJ_CALLFINALLY));

BasicBlock* const nextBlock = block->Next();

// Generate a call to the finally, like this:
// mov x0,qword ptr [fp + 10H] / sp // Load x0 with PSPSym, or sp if PSPSym is not used
// bl finally-funclet
Expand All @@ -2160,12 +2162,11 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
GetEmitter()->emitIns_Mov(INS_mov, EA_PTRSIZE, REG_R0, REG_SPBASE, /* canSkip */ false);
}
GetEmitter()->emitIns_J(INS_bl_local, block->GetTarget());

BasicBlock* const nextBlock = block->Next();

if (block->HasFlag(BBF_RETLESS_CALL))
{
GetEmitter()->emitIns_J(INS_bl_local, block->GetTarget());

// We have a retless call, and the last instruction generated was a call.
// If the next block is in a different EH region (or is the end of the code
// block), then we need to generate a breakpoint here (since it will never
Expand All @@ -2182,12 +2183,12 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
// Because of the way the flowgraph is connected, the liveness info for this one instruction
// after the call is not (can not be) correct in cases where a variable has a last use in the
// handler. So turn off GC reporting for this single instruction.
// handler. So turn off GC reporting once we execute the call and reenable after the jmp/nop
GetEmitter()->emitDisableGC();

BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
GetEmitter()->emitIns_J(INS_bl_local, block->GetTarget());

// Now go to where the finally funclet needs to return to.
BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
if (nextBlock->NextIs(finallyContinuation) && !compiler->fgInDifferentRegions(nextBlock, finallyContinuation))
{
// Fall-through.
Expand Down
13 changes: 7 additions & 6 deletions src/coreclr/jit/codegenloongarch64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1368,6 +1368,8 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
assert(block->KindIs(BBJ_CALLFINALLY));

BasicBlock* const nextBlock = block->Next();

// Generate a call to the finally, like this:
// mov a0,qword ptr [fp + 10H] / sp // Load a0 with PSPSym, or sp if PSPSym is not used
// bl finally-funclet
Expand All @@ -1382,12 +1384,11 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
GetEmitter()->emitIns_R_R_I(INS_ori, EA_PTRSIZE, REG_A0, REG_SPBASE, 0);
}
GetEmitter()->emitIns_J(INS_bl, block->GetTarget());

BasicBlock* const nextBlock = block->Next();

if (block->HasFlag(BBF_RETLESS_CALL))
{
GetEmitter()->emitIns_J(INS_bl, block->GetTarget());

// We have a retless call, and the last instruction generated was a call.
// If the next block is in a different EH region (or is the end of the code
// block), then we need to generate a breakpoint here (since it will never
Expand All @@ -1404,12 +1405,12 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
// Because of the way the flowgraph is connected, the liveness info for this one instruction
// after the call is not (can not be) correct in cases where a variable has a last use in the
// handler. So turn off GC reporting for this single instruction.
// handler. So turn off GC reporting once we execute the call and reenable after the jmp/nop
GetEmitter()->emitDisableGC();

BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
GetEmitter()->emitIns_J(INS_bl, block->GetTarget());

// Now go to where the finally funclet needs to return to.
BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
if (nextBlock->NextIs(finallyContinuation) && !compiler->fgInDifferentRegions(nextBlock, finallyContinuation))
{
// Fall-through.
Expand Down
13 changes: 7 additions & 6 deletions src/coreclr/jit/codegenriscv64.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1397,6 +1397,8 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
assert(block->KindIs(BBJ_CALLFINALLY));

BasicBlock* const nextBlock = block->Next();

// Generate a call to the finally, like this:
// mov a0,qword ptr [fp + 10H] / sp // Load a0 with PSPSym, or sp if PSPSym is not used
// jal finally-funclet
Expand All @@ -1411,12 +1413,11 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
GetEmitter()->emitIns_R_R_I(INS_ori, EA_PTRSIZE, REG_A0, REG_SPBASE, 0);
}
GetEmitter()->emitIns_J(INS_jal, block->GetTarget());

BasicBlock* const nextBlock = block->Next();

if (block->HasFlag(BBF_RETLESS_CALL))
{
GetEmitter()->emitIns_J(INS_jal, block->GetTarget());

// We have a retless call, and the last instruction generated was a call.
// If the next block is in a different EH region (or is the end of the code
// block), then we need to generate a breakpoint here (since it will never
Expand All @@ -1433,12 +1434,12 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
// Because of the way the flowgraph is connected, the liveness info for this one instruction
// after the call is not (can not be) correct in cases where a variable has a last use in the
// handler. So turn off GC reporting for this single instruction.
// handler. So turn off GC reporting once we execute the call and reenable after the jmp/nop
GetEmitter()->emitDisableGC();

BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
GetEmitter()->emitIns_J(INS_jal, block->GetTarget());

// Now go to where the finally funclet needs to return to.
BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
if (nextBlock->NextIs(finallyContinuation) && !compiler->fgInDifferentRegions(nextBlock, finallyContinuation))
{
// Fall-through.
Expand Down
8 changes: 5 additions & 3 deletions src/coreclr/jit/codegenxarch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,11 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
{
GetEmitter()->emitIns_R_S(ins_Load(TYP_I_IMPL), EA_PTRSIZE, REG_ARG_0, compiler->lvaPSPSym, 0);
}
GetEmitter()->emitIns_J(INS_call, block->GetTarget());

if (block->HasFlag(BBF_RETLESS_CALL))
{
GetEmitter()->emitIns_J(INS_call, block->GetTarget());

// We have a retless call, and the last instruction generated was a call.
// If the next block is in a different EH region (or is the end of the code
// block), then we need to generate a breakpoint here (since it will never
Expand All @@ -253,13 +254,14 @@ BasicBlock* CodeGen::genCallFinally(BasicBlock* block)
#ifndef JIT32_GCENCODER
// Because of the way the flowgraph is connected, the liveness info for this one instruction
// after the call is not (can not be) correct in cases where a variable has a last use in the
// handler. So turn off GC reporting for this single instruction.
// handler. So turn off GC reporting once we execute the call and reenable after the jmp/nop
GetEmitter()->emitDisableGC();
#endif // JIT32_GCENCODER

BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
GetEmitter()->emitIns_J(INS_call, block->GetTarget());

// Now go to where the finally funclet needs to return to.
BasicBlock* const finallyContinuation = nextBlock->GetFinallyContinuation();
if (nextBlock->NextIs(finallyContinuation) &&
!compiler->fgInDifferentRegions(nextBlock, finallyContinuation))
{
Expand Down
34 changes: 32 additions & 2 deletions src/coreclr/jit/emit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10485,7 +10485,27 @@ regMaskTP emitter::emitGetGCRegsKilledByNoGCCall(CorInfoHelpFunc helper)
}

#if !defined(JIT32_GCENCODER)
// Start a new instruction group that is not interruptible
//------------------------------------------------------------------------
// emitDisableGC: Requests that the following instruction groups are not GC-interruptible.
//
// Assumptions:
// A NoGC request can be closed by a coresponding emitEnableGC.
// Overlapping/nested NoGC requests are supported - when number of requests
// drops to zero the region is closed. This is not expected to be common.
// A completion of an epilog will close NoGC region in progress and clear the request count
// regardless of request nesting. If a block after the epilog needs to be no-GC, it needs
// to call emitDisableGC() again.
//
// Notes:
// The semantic of NoGC region is that once the first instruction executes,
// some tracking invariants are violated and GC cannot happen, until the execution
// of the last instruction in the region makes GC safe again.
// In particular - once the IP is on the first instruction, but not executed it yet,
// it is still safe to do GC.
// The only special case is when NoGC region is used for prologs/epilogs.
// In such case the GC info could be incorrect until the prolog completes and epilogs
// may have unwindability restrictions, so the first instruction cannot have GC.

void emitter::emitDisableGC()
{
assert(emitNoGCRequestCount < 10); // We really shouldn't have many nested "no gc" requests.
Expand Down Expand Up @@ -10514,7 +10534,17 @@ void emitter::emitDisableGC()
}
}

// Start a new instruction group that is interruptible
//------------------------------------------------------------------------
// emitEnableGC(): Removes a request that the following instruction groups are not GC-interruptible.
//
// Assumptions:
// We should have nonzero count of NoGC requests.
// "emitEnableGC()" reduces the number of requests by 1.
// If the number of requests goes to 0, the subsequent instruction groups are GC-interruptible.
//
// Notes:
// See emitDisableGC for more details.

void emitter::emitEnableGC()
{
assert(emitNoGCRequestCount > 0);
Expand Down
8 changes: 6 additions & 2 deletions src/coreclr/jit/emitinl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,9 +589,13 @@ bool emitter::emitGenNoGCLst(Callback& cb)
{
for (insGroup* ig = emitIGlist; ig; ig = ig->igNext)
{
if (ig->igFlags & IGF_NOGCINTERRUPT)
if ((ig->igFlags & IGF_NOGCINTERRUPT) && ig->igSize > 0)
{
if (!cb(ig->igFuncIdx, ig->igOffs, ig->igSize))
emitter::instrDesc* id = emitFirstInstrDesc(ig->igData);
assert(id != nullptr);
assert(id->idCodeSize() > 0);
if (!cb(ig->igFuncIdx, ig->igOffs, ig->igSize, id->idCodeSize(),
ig->igFlags & (IGF_FUNCLET_PROLOG | IGF_FUNCLET_EPILOG | IGF_EPILOG)))
{
return false;
}
Expand Down
55 changes: 33 additions & 22 deletions src/coreclr/jit/gcencode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4011,41 +4011,53 @@ void GCInfo::gcInfoBlockHdrSave(GcInfoEncoder* gcInfoEncoder, unsigned methodSiz
//
// Encoder should be either GcInfoEncoder or GcInfoEncoderWithLogging
//
struct InterruptibleRangeReporter
class InterruptibleRangeReporter
{
unsigned prevStart;
Encoder* gcInfoEncoderWithLog;
unsigned m_uninterruptibleEnd;
Encoder* m_gcInfoEncoder;

InterruptibleRangeReporter(unsigned _prevStart, Encoder* _gcInfo)
: prevStart(_prevStart)
, gcInfoEncoderWithLog(_gcInfo)
public:
InterruptibleRangeReporter(unsigned prologSize, Encoder* gcInfo)
: m_uninterruptibleEnd(prologSize)
, m_gcInfoEncoder(gcInfo)
{
}

// This callback is called for each insGroup marked with
// IGF_NOGCINTERRUPT (currently just prologs and epilogs).
// This callback is called for each insGroup marked with IGF_NOGCINTERRUPT.
// Report everything between the previous region and the current
// region as interruptible.

bool operator()(unsigned igFuncIdx, unsigned igOffs, unsigned igSize)
bool operator()(
unsigned igFuncIdx, unsigned igOffs, unsigned igSize, unsigned firstInstrSize, bool isInPrologOrEpilog)
{
if (igOffs < prevStart)
if (igOffs < m_uninterruptibleEnd)
{
// We're still in the main method prolog, which has already
// had it's interruptible range reported.
// We're still in the main method prolog, which we know is not interruptible.
assert(igFuncIdx == 0);
assert(igOffs + igSize <= prevStart);
assert(igOffs + igSize <= m_uninterruptibleEnd);
return true;
}

assert(igOffs >= prevStart);
if (igOffs > prevStart)
assert(igOffs >= m_uninterruptibleEnd);
if (igOffs > m_uninterruptibleEnd)
{
gcInfoEncoderWithLog->DefineInterruptibleRange(prevStart, igOffs - prevStart);
// Once the first instruction in IG executes, we cannot have GC.
// But it is ok to have GC while the IP is on the first instruction, unless we are in prolog/epilog.
unsigned interruptibleEnd = igOffs;
if (!isInPrologOrEpilog)
{
interruptibleEnd += firstInstrSize;
}
m_gcInfoEncoder->DefineInterruptibleRange(m_uninterruptibleEnd, interruptibleEnd - m_uninterruptibleEnd);
}
prevStart = igOffs + igSize;
m_uninterruptibleEnd = igOffs + igSize;
return true;
}

unsigned UninterruptibleEnd()
{
return m_uninterruptibleEnd;
}
};

void GCInfo::gcMakeRegPtrTable(
Expand Down Expand Up @@ -4396,17 +4408,16 @@ void GCInfo::gcMakeRegPtrTable(
{
assert(prologSize <= codeSize);

// Now exempt any other region marked as IGF_NOGCINTERRUPT
// Currently just prologs and epilogs.
// Now exempt any region marked as IGF_NOGCINTERRUPT

InterruptibleRangeReporter reporter(prologSize, gcInfoEncoderWithLog);
compiler->GetEmitter()->emitGenNoGCLst(reporter);
prologSize = reporter.prevStart;
unsigned uninterruptibleEnd = reporter.UninterruptibleEnd();

// Report any remainder
if (prologSize < codeSize)
if (uninterruptibleEnd < codeSize)
{
gcInfoEncoderWithLog->DefineInterruptibleRange(prologSize, codeSize - prologSize);
gcInfoEncoderWithLog->DefineInterruptibleRange(uninterruptibleEnd, codeSize - uninterruptibleEnd);
}
}
}
Expand Down

0 comments on commit 9ef950b

Please sign in to comment.