Skip to content

Commit

Permalink
Fixes for CFG dispatcher on ARM64 (#65127)
Browse files Browse the repository at this point in the history
* Do not run jit-cfg on macOS

jit-cfg includes a GCStress scenario which is not supported on macOS
x64. Rather than excluding it just there, just exclude all of macOS
since CFG is a Windows only feature anyway.

I am still keeping the Linux-x64/Linux-arm64 jobs as we produce
different IR for arguments on these ABIs and I want to keep the CFG
handling working in general for all the forms of IR we can produce
related to calls.

* Fix CFG dispatcher register in VM stub

* Change GS cookie check temp registers

These registers are trashed in the epilog on arm64 for GS cookie checks,
but x9 might conflict with the argument used by the dispatch helper.
Change the temp registers to ip0 and ip1 and add some debug checking to
get a nice assert in the future if this happens.
  • Loading branch information
jakobbotsch authored Feb 15, 2022
1 parent bc14327 commit a25aa66
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 15 deletions.
4 changes: 0 additions & 4 deletions eng/pipelines/coreclr/jit-cfg.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ jobs:
jobTemplate: /eng/pipelines/common/build-coreclr-and-libraries-job.yml
buildConfig: checked
platforms:
- OSX_arm64
- OSX_x64
- Linux_arm64
- Linux_x64
- windows_arm64
Expand All @@ -39,8 +37,6 @@ jobs:
jobTemplate: /eng/pipelines/common/templates/runtimes/run-test-job.yml
buildConfig: checked
platforms:
- OSX_arm64
- OSX_x64
- Linux_arm64
- Linux_x64
- windows_arm64
Expand Down
51 changes: 44 additions & 7 deletions src/coreclr/jit/codegenarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3331,6 +3331,35 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
{
sigInfo = call->callSig;
}

if (call->IsFastTailCall())
{
regMaskTP trashedByEpilog = RBM_CALLEE_SAVED;

// The epilog may use and trash REG_GSCOOKIE_TMP_0/1. Make sure we have no
// non-standard args that may be trash if this is a tailcall.
if (compiler->getNeedsGSSecurityCookie())
{
trashedByEpilog |= genRegMask(REG_GSCOOKIE_TMP_0);
trashedByEpilog |= genRegMask(REG_GSCOOKIE_TMP_1);
}

for (unsigned i = 0; i < call->fgArgInfo->ArgCount(); i++)
{
fgArgTabEntry* entry = call->fgArgInfo->GetArgEntry(i);
for (unsigned j = 0; j < entry->numRegs; j++)
{
regNumber reg = entry->GetRegNum(j);
if ((trashedByEpilog & genRegMask(reg)) != 0)
{
JITDUMP("Tail call node:\n");
DISPTREE(call);
JITDUMP("Register used: %s\n", getRegName(reg));
assert(!"Argument to tailcall may be trashed by epilog");
}
}
}
}
#endif // DEBUG
CORINFO_METHOD_HANDLE methHnd;
GenTree* target = getCallTarget(call, &methHnd);
Expand Down Expand Up @@ -3367,20 +3396,28 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
}
else
{
// If we have no target and this is a call with indirection cell
// then we do an optimization where we load the call address directly
// from the indirection cell instead of duplicating the tree.
// In BuildCall we ensure that get an extra register for the purpose.
regNumber indirCellReg = getCallIndirectionCellReg(call);
if (indirCellReg != REG_NA)
// If we have no target and this is a call with indirection cell then
// we do an optimization where we load the call address directly from
// the indirection cell instead of duplicating the tree. In BuildCall
// we ensure that get an extra register for the purpose. Note that for
// CFG the call might have changed to
// CORINFO_HELP_DISPATCH_INDIRECT_CALL in which case we still have the
// indirection cell but we should not try to optimize.
regNumber callThroughIndirReg = REG_NA;
if (!call->IsHelperCall(compiler, CORINFO_HELP_DISPATCH_INDIRECT_CALL))
{
callThroughIndirReg = getCallIndirectionCellReg(call);
}

if (callThroughIndirReg != REG_NA)
{
assert(call->IsR2ROrVirtualStubRelativeIndir());
regNumber targetAddrReg = call->GetSingleTempReg();
// For fast tailcalls we have already loaded the call target when processing the call node.
if (!call->IsFastTailCall())
{
GetEmitter()->emitIns_R_R(ins_Load(TYP_I_IMPL), emitActualTypeSize(TYP_I_IMPL), targetAddrReg,
indirCellReg);
callThroughIndirReg);
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/targetarm64.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@
#define CALLEE_SAVED_FLOAT_MAXSZ (CNT_CALLEE_SAVED_FLOAT * FPSAVE_REGSIZE_BYTES)

// Temporary registers used for the GS cookie check.
#define REG_GSCOOKIE_TMP_0 REG_R9
#define REG_GSCOOKIE_TMP_1 REG_R10
#define REG_GSCOOKIE_TMP_0 REG_IP0
#define REG_GSCOOKIE_TMP_1 REG_IP1

// register to hold shift amount; no special register is required on ARM64.
#define REG_SHIFT REG_NA
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/arm64/asmhelpers.S
Original file line number Diff line number Diff line change
Expand Up @@ -1026,5 +1026,5 @@ LEAF_ENTRY JIT_ValidateIndirectCall, _TEXT
LEAF_END JIT_ValidateIndirectCall, _TEXT

LEAF_ENTRY JIT_DispatchIndirectCall, _TEXT
br x15
br x9
LEAF_END JIT_DispatchIndirectCall, _TEXT
2 changes: 1 addition & 1 deletion src/coreclr/vm/arm64/asmhelpers.asm
Original file line number Diff line number Diff line change
Expand Up @@ -1440,7 +1440,7 @@ __HelperNakedFuncName SETS "$helper":CC:"Naked"
LEAF_END

LEAF_ENTRY JIT_DispatchIndirectCall
br x15
br x9
LEAF_END

; Must be at very end of file
Expand Down

0 comments on commit a25aa66

Please sign in to comment.