From a25aa6641b605f1ad28c75cddcfa204f8153abe3 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 15 Feb 2022 11:28:17 +0100 Subject: [PATCH] Fixes for CFG dispatcher on ARM64 (#65127) * 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. --- eng/pipelines/coreclr/jit-cfg.yml | 4 --- src/coreclr/jit/codegenarmarch.cpp | 51 +++++++++++++++++++++++++---- src/coreclr/jit/targetarm64.h | 4 +-- src/coreclr/vm/arm64/asmhelpers.S | 2 +- src/coreclr/vm/arm64/asmhelpers.asm | 2 +- 5 files changed, 48 insertions(+), 15 deletions(-) diff --git a/eng/pipelines/coreclr/jit-cfg.yml b/eng/pipelines/coreclr/jit-cfg.yml index 6b30445c2363c..ed1a26720856e 100644 --- a/eng/pipelines/coreclr/jit-cfg.yml +++ b/eng/pipelines/coreclr/jit-cfg.yml @@ -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 @@ -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 diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 0ece8cf945a5e..0a2ed92ff78b4 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -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); @@ -3367,12 +3396,20 @@ 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(); @@ -3380,7 +3417,7 @@ void CodeGen::genCallInstruction(GenTreeCall* call) if (!call->IsFastTailCall()) { GetEmitter()->emitIns_R_R(ins_Load(TYP_I_IMPL), emitActualTypeSize(TYP_I_IMPL), targetAddrReg, - indirCellReg); + callThroughIndirReg); } else { diff --git a/src/coreclr/jit/targetarm64.h b/src/coreclr/jit/targetarm64.h index 1901d21cef4fa..c1bd6f838490c 100644 --- a/src/coreclr/jit/targetarm64.h +++ b/src/coreclr/jit/targetarm64.h @@ -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 diff --git a/src/coreclr/vm/arm64/asmhelpers.S b/src/coreclr/vm/arm64/asmhelpers.S index 941af8f3d00ca..ac85299cf1ad3 100644 --- a/src/coreclr/vm/arm64/asmhelpers.S +++ b/src/coreclr/vm/arm64/asmhelpers.S @@ -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 diff --git a/src/coreclr/vm/arm64/asmhelpers.asm b/src/coreclr/vm/arm64/asmhelpers.asm index d73b8f5ff6ff7..f383c577c3ae1 100644 --- a/src/coreclr/vm/arm64/asmhelpers.asm +++ b/src/coreclr/vm/arm64/asmhelpers.asm @@ -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