Skip to content

Commit

Permalink
Fix unwind info when fake-splitting; add job to runtime-jit-experimen…
Browse files Browse the repository at this point in the history
…tal (#69922)

Fake-splitting currently breaks stack walks by not generating unwind
info for cold code. This commit implements unwind info on x86/64 when
fake-splitting by generating unwind info for the combined hot/cold
section just once, rather than generating unwind info for the separate
sections.

For reasons to be investigated, this implementation does not work when
the code sections are separated by an arbitrary buffer (such as the 4KB
buffer previously used). Thus, the buffer has been removed from the
fake-splitting implementation: Now, the hot and cold sections are
placed contiguously in memory, but the JIT continues to behave as if
they are arbitrarily far away (for example, by using long branches
between sections).

Following this fix, fake-splitting no longer requires the GC to be
suppressed by setting `COMPlus_GCgen0size=1000000`. A test job
has been added to `runtime-jit-experimental` to ensure
fake/stress-splitting does not regress.
  • Loading branch information
Aman Khalid authored Jun 7, 2022
1 parent 0c4ee9e commit 08b3170
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 42 deletions.
1 change: 1 addition & 0 deletions eng/pipelines/common/templates/runtimes/run-test-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,7 @@ jobs:
- jitosr_stress
- jitosr_pgo
- jitosr_stress_random
- jit_stress_splitting
- jitpartialcompilation
- jitpartialcompilation_osr
- jitpartialcompilation_osr_pgo
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -8000,6 +8000,10 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
void unwindReserveFuncHelper(FuncInfoDsc* func, bool isHotCode);
void unwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode, void* pColdCode, bool isHotCode);

#ifdef DEBUG
void fakeUnwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode);
#endif // DEBUG

#endif // TARGET_AMD64 || (TARGET_X86 && FEATURE_EH_FUNCLETS)

UNATIVE_OFFSET unwindGetCurrentOffset(FuncInfoDsc* func);
Expand Down
27 changes: 6 additions & 21 deletions src/coreclr/jit/ee_il_dll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1125,14 +1125,13 @@ void Compiler::eeDispLineInfos()
void Compiler::eeAllocMem(AllocMemArgs* args)
{
#ifdef DEBUG
// Fake splitting implementation: hot section = hot code + 4K buffer + cold code
const UNATIVE_OFFSET hotSizeRequest = args->hotCodeSize;
const UNATIVE_OFFSET coldSizeRequest = args->coldCodeSize;
const UNATIVE_OFFSET fakeSplittingBuffer = 4096;
const UNATIVE_OFFSET hotSizeRequest = args->hotCodeSize;
const UNATIVE_OFFSET coldSizeRequest = args->coldCodeSize;

// Fake splitting implementation: place hot/cold code in contiguous section
if (JitConfig.JitFakeProcedureSplitting() && (coldSizeRequest > 0))
{
args->hotCodeSize = hotSizeRequest + fakeSplittingBuffer + coldSizeRequest;
args->hotCodeSize = hotSizeRequest + coldSizeRequest;
args->coldCodeSize = 0;
}
#endif
Expand All @@ -1143,8 +1142,8 @@ void Compiler::eeAllocMem(AllocMemArgs* args)
if (JitConfig.JitFakeProcedureSplitting() && (coldSizeRequest > 0))
{
// Fix up hot/cold code pointers
args->coldCodeBlock = ((BYTE*)args->hotCodeBlock) + hotSizeRequest + fakeSplittingBuffer;
args->coldCodeBlockRW = ((BYTE*)args->hotCodeBlockRW) + hotSizeRequest + fakeSplittingBuffer;
args->coldCodeBlock = ((BYTE*)args->hotCodeBlock) + hotSizeRequest;
args->coldCodeBlockRW = ((BYTE*)args->hotCodeBlockRW) + hotSizeRequest;

// Reset args' hot/cold code sizes in case caller reads them later
args->hotCodeSize = hotSizeRequest;
Expand All @@ -1161,13 +1160,6 @@ void Compiler::eeReserveUnwindInfo(bool isFunclet, bool isColdCode, ULONG unwind
printf("reserveUnwindInfo(isFunclet=%s, isColdCode=%s, unwindSize=0x%x)\n", isFunclet ? "true" : "false",
isColdCode ? "true" : "false", unwindSize);
}

// Fake splitting currently does not handle unwind info for cold code
if (isColdCode && JitConfig.JitFakeProcedureSplitting())
{
JITDUMP("reserveUnwindInfo for cold code with JitFakeProcedureSplitting enabled: ignoring cold unwind info\n");
return;
}
#endif // DEBUG

if (info.compMatchedVM)
Expand Down Expand Up @@ -1207,13 +1199,6 @@ void Compiler::eeAllocUnwindInfo(BYTE* pHotCode,
}
printf(")\n");
}

// Fake splitting currently does not handle unwind info for cold code
if (pColdCode && JitConfig.JitFakeProcedureSplitting())
{
JITDUMP("allocUnwindInfo for cold code with JitFakeProcedureSplitting enabled: ignoring cold unwind info\n");
return;
}
#endif // DEBUG

if (info.compMatchedVM)
Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/jit/emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6638,10 +6638,13 @@ unsigned emitter::emitEndCodeGen(Compiler* comp,
{
// The allocated chunk is bigger than used, fill in unused space in it.
unsigned unusedSize = allocatedHotCodeSize - emitCurCodeOffs(cp);
BYTE* cpRW = cp + writeableOffset;
for (unsigned i = 0; i < unusedSize; ++i)
{
*cp++ = DEFAULT_CODE_BUFFER_INIT;
*cpRW++ = DEFAULT_CODE_BUFFER_INIT;
}

cp = cpRW - writeableOffset;
assert(allocatedHotCodeSize == emitCurCodeOffs(cp));
}
}
Expand Down
5 changes: 0 additions & 5 deletions src/coreclr/jit/jitconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,6 @@ CONFIG_INTEGER(JitDumpInlinePhases, W("JitDumpInlinePhases"), 1) // Dump inline
CONFIG_METHODSET(JitEHDump, W("JitEHDump")) // Dump the EH table for the method, as reported to the VM
CONFIG_METHODSET(JitExclude, W("JitExclude"))
CONFIG_INTEGER(JitFakeProcedureSplitting, W("JitFakeProcedureSplitting"), 0) // Do code splitting independent of VM.
// For now, this disables unwind info for
// cold sections, breaking stack walks.
// Set COMPlus_GCgen0size=1000000 to avoid
// running the GC, which requires
// stack-walking.
CONFIG_METHODSET(JitForceProcedureSplitting, W("JitForceProcedureSplitting"))
CONFIG_METHODSET(JitGCDump, W("JitGCDump"))
CONFIG_METHODSET(JitDebugDump, W("JitDebugDump"))
Expand Down
55 changes: 48 additions & 7 deletions src/coreclr/jit/unwindamd64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,21 @@ void Compiler::unwindReserve()
//
void Compiler::unwindReserveFunc(FuncInfoDsc* func)
{
unwindReserveFuncHelper(func, true);

if (fgFirstColdBlock != nullptr)
#ifdef DEBUG
if (JitConfig.JitFakeProcedureSplitting() && (fgFirstColdBlock != nullptr))
{
unwindReserveFuncHelper(func, false);
assert(func->funKind == FUNC_ROOT); // No fake-splitting of funclets.
unwindReserveFuncHelper(func, true);
}
else
#endif // DEBUG
{
unwindReserveFuncHelper(func, true);

if (fgFirstColdBlock != nullptr)
{
unwindReserveFuncHelper(func, false);
}
}
}

Expand Down Expand Up @@ -880,12 +890,43 @@ void Compiler::unwindEmitFunc(FuncInfoDsc* func, void* pHotCode, void* pColdCode
static_assert_no_msg(FUNC_HANDLER == (FuncKind)CORJIT_FUNC_HANDLER);
static_assert_no_msg(FUNC_FILTER == (FuncKind)CORJIT_FUNC_FILTER);

unwindEmitFuncHelper(func, pHotCode, pColdCode, true);
#ifdef DEBUG
if (JitConfig.JitFakeProcedureSplitting() && (pColdCode != nullptr))
{
fakeUnwindEmitFuncHelper(func, pHotCode);
}
else
#endif // DEBUG
{
unwindEmitFuncHelper(func, pHotCode, pColdCode, true);

if (pColdCode != nullptr)
{
unwindEmitFuncHelper(func, pHotCode, pColdCode, false);
}
}
}

#ifdef DEBUG
void Compiler::fakeUnwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode)
{
assert(fgFirstColdBlock != nullptr);
assert(func->funKind == FUNC_ROOT); // No fake-splitting of funclets.

if (pColdCode != nullptr)
const UNATIVE_OFFSET startOffset = 0;
const UNATIVE_OFFSET endOffset = info.compNativeCodeSize;
const DWORD unwindCodeBytes = sizeof(func->unwindCodes) - func->unwindCodeSlot;
BYTE* pUnwindBlock = &func->unwindCodes[func->unwindCodeSlot];

if (opts.dspUnwind)
{
unwindEmitFuncHelper(func, pHotCode, pColdCode, false);
DumpUnwindInfo(true, startOffset, endOffset, (const UNWIND_INFO* const)pUnwindBlock);
}

// Pass pColdCode = nullptr; VM allocs unwind info for combined hot/cold section
eeAllocUnwindInfo((BYTE*)pHotCode, nullptr, startOffset, endOffset, unwindCodeBytes, pUnwindBlock,
(CorJitFuncKind)func->funKind);
}
#endif // DEBUG

#endif // TARGET_AMD64
54 changes: 46 additions & 8 deletions src/coreclr/jit/unwindx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,21 @@ void Compiler::unwindEmit(void* pHotCode, void* pColdCode)
//
void Compiler::unwindReserveFunc(FuncInfoDsc* func)
{
unwindReserveFuncHelper(func, true);

if (fgFirstColdBlock != nullptr)
#ifdef DEBUG
if (JitConfig.JitFakeProcedureSplitting() && (fgFirstColdBlock != nullptr))
{
unwindReserveFuncHelper(func, false);
assert(func->funKind == FUNC_ROOT); // No fake-splitting of funclets.
unwindReserveFuncHelper(func, true);
}
else
#endif // DEBUG
{
unwindReserveFuncHelper(func, true);

if (fgFirstColdBlock != nullptr)
{
unwindReserveFuncHelper(func, false);
}
}
}

Expand Down Expand Up @@ -154,11 +164,20 @@ void Compiler::unwindEmitFunc(FuncInfoDsc* func, void* pHotCode, void* pColdCode
static_assert_no_msg(FUNC_HANDLER == (FuncKind)CORJIT_FUNC_HANDLER);
static_assert_no_msg(FUNC_FILTER == (FuncKind)CORJIT_FUNC_FILTER);

unwindEmitFuncHelper(func, pHotCode, pColdCode, true);

if (pColdCode != nullptr)
#ifdef DEBUG
if (JitConfig.JitFakeProcedureSplitting() && (pColdCode != nullptr))
{
fakeUnwindEmitFuncHelper(func, pHotCode);
}
else
#endif // DEBUG
{
unwindEmitFuncHelper(func, pHotCode, pColdCode, false);
unwindEmitFuncHelper(func, pHotCode, pColdCode, true);

if (pColdCode != nullptr)
{
unwindEmitFuncHelper(func, pHotCode, pColdCode, false);
}
}
}

Expand Down Expand Up @@ -256,4 +275,23 @@ void Compiler::unwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode, void* pCo
eeAllocUnwindInfo((BYTE*)pHotCode, (BYTE*)pColdCode, startOffset, endOffset, sizeof(UNWIND_INFO),
(BYTE*)&unwindInfo, (CorJitFuncKind)func->funKind);
}

#ifdef DEBUG
void Compiler::fakeUnwindEmitFuncHelper(FuncInfoDsc* func, void* pHotCode)
{
assert(fgFirstColdBlock != nullptr);
assert(func->funKind == FUNC_ROOT); // No fake-splitting of funclets.

const UNATIVE_OFFSET startOffset = 0;
const UNATIVE_OFFSET endOffset = info.compNativeCodeSize;

UNWIND_INFO unwindInfo;
unwindInfo.FunctionLength = (ULONG)(endOffset);

// Pass pColdCode = nullptr; VM allocs unwind info for combined hot/cold section
eeAllocUnwindInfo((BYTE*)pHotCode, nullptr, startOffset, endOffset, sizeof(UNWIND_INFO), (BYTE*)&unwindInfo,
(CorJitFuncKind)func->funKind);
}
#endif // DEBUG

#endif // FEATURE_EH_FUNCLETS
3 changes: 3 additions & 0 deletions src/tests/Common/testenvironment.proj
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@
COMPlus_HeapVerify;
COMPlus_JITMinOpts;
COMPlus_JitELTHookEnabled;
COMPlus_JitFakeProcedureSplitting;
COMPlus_JitStress;
COMPlus_JitStressProcedureSplitting;
COMPlus_JitStressRegs;
COMPlus_TailcallStress;
COMPlus_ReadyToRun;
Expand Down Expand Up @@ -188,6 +190,7 @@
<TestEnvironment Include="jitosr" TC_OnStackReplacement="1" TC_QuickJitForLoops="1" TieredCompilation="1" />
<TestEnvironment Include="jitosr_stress" TC_OnStackReplacement="1" TC_QuickJitForLoops="1" TC_OnStackReplacement_InitialCounter="1" OSR_HitLimit="1" TieredCompilation="1" />
<TestEnvironment Include="jitosr_stress_random" TC_OnStackReplacement="1" TC_QuickJitForLoops="1" TC_OnStackReplacement_InitialCounter="1" OSR_HitLimit="2" TieredCompilation="1" JitRandomOnStackReplacement="15"/>
<TestEnvironment Include="jit_stress_splitting" JitFakeProcedureSplitting="1" JitStressProcedureSplitting="1" />
<TestEnvironment Include="jitosr_pgo" TC_OnStackReplacement="1" TC_QuickJitForLoops="1" TieredCompilation="1" TieredPGO="1" />
<TestEnvironment Include="jitpartialcompilation" TC_PartialCompilation="1" TC_QuickJitForLoops="1" TieredCompilation="1" />
<TestEnvironment Include="jitpartialcompilation_pgo" TC_PartialCompilation="1" TC_QuickJitForLoops="1" TieredCompilation="1" TieredPGO="1" />
Expand Down

0 comments on commit 08b3170

Please sign in to comment.