From 9b0959cf162b11b2fcdb78198ea320ae689d5fcc Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 13 Oct 2023 01:02:15 +0200 Subject: [PATCH 1/6] Make FailFast cold for GS cookies --- src/coreclr/jit/codegenxarch.cpp | 7 ++----- src/coreclr/jit/compiler.hpp | 5 +++-- src/coreclr/jit/flowgraph.cpp | 10 ++++++++++ src/coreclr/jit/gentree.h | 1 + src/coreclr/jit/gschecks.cpp | 2 ++ 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 0264d9e3e85868..78a521e3222eca 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -195,11 +195,8 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg) GetEmitter()->emitIns_S_R(INS_cmp, EA_PTRSIZE, regGSCheck, compiler->lvaGSSecurityCookie, 0); } - BasicBlock* gsCheckBlk = genCreateTempLabel(); - inst_JMP(EJ_je, gsCheckBlk); - genEmitHelperCall(CORINFO_HELP_FAIL_FAST, 0, EA_UNKNOWN); - genDefineTempLabel(gsCheckBlk); - + Compiler::AddCodeDsc* codeDsc = compiler->fgFindExcptnTarget(SpecialCodeKind::SCK_FAIL_FAST, 0); + inst_JMP(EJ_jne, codeDsc->acdDstBlk); genPopRegs(pushedRegs, byrefPushedRegs, norefPushedRegs); } diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index a3bed975f9dfaf..3e0f7a1b8bb905 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -3146,6 +3146,7 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block) if (!((call->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_RNGCHKFAIL)) || (call->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROWDIVZERO)) || + (call->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_FAIL_FAST)) || (call->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROW_ARGUMENTEXCEPTION)) || (call->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_THROW_ARGUMENTOUTOFRANGEEXCEPTION)) || (call->AsCall()->gtCallMethHnd == eeFindHelper(CORINFO_HELP_OVERFLOW)))) @@ -3162,7 +3163,7 @@ inline bool Compiler::fgIsThrowHlpBlk(BasicBlock* block) if (block == add->acdDstBlk) { return add->acdKind == SCK_RNGCHK_FAIL || add->acdKind == SCK_DIV_BY_ZERO || add->acdKind == SCK_OVERFLOW || - add->acdKind == SCK_ARG_EXCPN || add->acdKind == SCK_ARG_RNG_EXCPN; + add->acdKind == SCK_ARG_EXCPN || add->acdKind == SCK_ARG_RNG_EXCPN || add->acdKind == SCK_FAIL_FAST; } } @@ -3187,7 +3188,7 @@ inline unsigned Compiler::fgThrowHlpBlkStkLevel(BasicBlock* block) // Compute assert cond separately as assert macro cannot have conditional compilation directives. bool cond = (add->acdKind == SCK_RNGCHK_FAIL || add->acdKind == SCK_DIV_BY_ZERO || add->acdKind == SCK_OVERFLOW || - add->acdKind == SCK_ARG_EXCPN || add->acdKind == SCK_ARG_RNG_EXCPN); + add->acdKind == SCK_ARG_EXCPN || add->acdKind == SCK_ARG_RNG_EXCPN || add->acdKind == SCK_FAIL_FAST); assert(cond); // TODO: bbTgtStkDepth is DEBUG-only. diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 3f7a622716024a..14dd9740c9963a 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3592,6 +3592,8 @@ unsigned Compiler::acdHelper(SpecialCodeKind codeKind) return CORINFO_HELP_THROWDIVZERO; case SCK_ARITH_EXCPN: return CORINFO_HELP_OVERFLOW; + case SCK_FAIL_FAST: + return CORINFO_HELP_FAIL_FAST; default: assert(!"Bad codeKind"); return 0; @@ -3628,6 +3630,7 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special BBJ_THROW, // SCK_ARITH_EXCP, SCK_OVERFLOW BBJ_THROW, // SCK_ARG_EXCPN BBJ_THROW, // SCK_ARG_RNG_EXCPN + BBJ_THROW, // SCK_FAIL_FAST }; noway_assert(sizeof(jumpKinds) == SCK_COUNT); // sanity check @@ -3703,6 +3706,9 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special case SCK_ARG_RNG_EXCPN: msg = " for ARG_RNG_EXCPN"; break; + case SCK_FAIL_FAST: + msg = " for FAIL_FAST"; + break; default: msg = " for ??"; break; @@ -3751,6 +3757,10 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special helper = CORINFO_HELP_THROW_ARGUMENTOUTOFRANGEEXCEPTION; break; + case SCK_FAIL_FAST: + helper = CORINFO_HELP_FAIL_FAST; + break; + default: noway_assert(!"unexpected code addition kind"); return nullptr; diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 31180df37a00da..0e415a555b53fb 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -60,6 +60,7 @@ enum SpecialCodeKind SCK_OVERFLOW = SCK_ARITH_EXCPN, // target on overflow SCK_ARG_EXCPN, // target on ArgumentException (currently used only for SIMD intrinsics) SCK_ARG_RNG_EXCPN, // target on ArgumentOutOfRangeException (currently used only for SIMD intrinsics) + SCK_FAIL_FAST, // target for fail fast exception SCK_COUNT }; diff --git a/src/coreclr/jit/gschecks.cpp b/src/coreclr/jit/gschecks.cpp index 0953920d6192e3..228d14d48b8871 100644 --- a/src/coreclr/jit/gschecks.cpp +++ b/src/coreclr/jit/gschecks.cpp @@ -30,6 +30,8 @@ PhaseStatus Compiler::gsPhase() unsigned const prevBBCount = fgBBcount; gsGSChecksInitCookie(); + fgAddCodeRef(fgLastBB, 0, SCK_FAIL_FAST); + if (compGSReorderStackLayout) { gsCopyShadowParams(); From 9d82cda6a2c094ab2852789170289a47400537e3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 13 Oct 2023 01:21:17 +0200 Subject: [PATCH 2/6] ARM, ARM64, LA, RISC-V --- src/coreclr/jit/codegenarmarch.cpp | 7 ++----- src/coreclr/jit/codegenloongarch64.cpp | 8 ++------ src/coreclr/jit/codegenriscv64.cpp | 8 ++------ 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index fd0b2de289b613..7bcd66bfb4e0f0 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -631,11 +631,8 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg) // Compare with the GC cookie constant GetEmitter()->emitIns_R_R(INS_cmp, EA_PTRSIZE, regGSConst, regGSValue); - BasicBlock* gsCheckBlk = genCreateTempLabel(); - inst_JMP(EJ_eq, gsCheckBlk); - // regGSConst and regGSValue aren't needed anymore, we can use them for helper call - genEmitHelperCall(CORINFO_HELP_FAIL_FAST, 0, EA_UNKNOWN, regGSConst); - genDefineTempLabel(gsCheckBlk); + Compiler::AddCodeDsc* codeDsc = compiler->fgFindExcptnTarget(SpecialCodeKind::SCK_FAIL_FAST, 0); + inst_JMP(EJ_ne, codeDsc->acdDstBlk); } //--------------------------------------------------------------------- diff --git a/src/coreclr/jit/codegenloongarch64.cpp b/src/coreclr/jit/codegenloongarch64.cpp index 44edc3fae0fee1..d77532365e6610 100644 --- a/src/coreclr/jit/codegenloongarch64.cpp +++ b/src/coreclr/jit/codegenloongarch64.cpp @@ -5192,12 +5192,8 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg) GetEmitter()->emitIns_R_S(INS_ld_d, EA_PTRSIZE, regGSValue, compiler->lvaGSSecurityCookie, 0); // Compare with the GC cookie constant - BasicBlock* gsCheckBlk = genCreateTempLabel(); - GetEmitter()->emitIns_J_cond_la(INS_beq, gsCheckBlk, regGSConst, regGSValue); - - // regGSConst and regGSValue aren't needed anymore, we can use them for helper call - genEmitHelperCall(CORINFO_HELP_FAIL_FAST, 0, EA_UNKNOWN, regGSConst); - genDefineTempLabel(gsCheckBlk); + Compiler::AddCodeDsc* codeDsc = compiler->fgFindExcptnTarget(SpecialCodeKind::SCK_FAIL_FAST, 0); + GetEmitter()->emitIns_J_cond_la(INS_bne, codeDsc->acdDstBlk, regGSConst, regGSValue); } //--------------------------------------------------------------------- diff --git a/src/coreclr/jit/codegenriscv64.cpp b/src/coreclr/jit/codegenriscv64.cpp index 8b9fb2b8ddf5b5..de240bdca5f5fd 100644 --- a/src/coreclr/jit/codegenriscv64.cpp +++ b/src/coreclr/jit/codegenriscv64.cpp @@ -4958,12 +4958,8 @@ void CodeGen::genEmitGSCookieCheck(bool pushReg) GetEmitter()->emitIns_R_S(INS_ld, EA_PTRSIZE, regGSValue, compiler->lvaGSSecurityCookie, 0); // Compare with the GC cookie constant - BasicBlock* gsCheckBlk = genCreateTempLabel(); - GetEmitter()->emitIns_J_cond_la(INS_beq, gsCheckBlk, regGSConst, regGSValue); - - // regGSConst and regGSValue aren't needed anymore, we can use them for helper call - genEmitHelperCall(CORINFO_HELP_FAIL_FAST, 0, EA_UNKNOWN, regGSConst); - genDefineTempLabel(gsCheckBlk); + Compiler::AddCodeDsc* codeDsc = compiler->fgFindExcptnTarget(SpecialCodeKind::SCK_FAIL_FAST, 0); + GetEmitter()->emitIns_J_cond_la(INS_bne, codeDsc->acdDstBlk, regGSConst, regGSValue); } //--------------------------------------------------------------------- From b2abdffebcd62a83fc31ea75ab8617ac9329e0a5 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 13 Oct 2023 01:34:29 +0200 Subject: [PATCH 3/6] Clean up --- src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/flowgraph.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index cc9819a913fe4b..8b633d741f27dd 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6127,7 +6127,6 @@ class Compiler AddCodeDsc* fgAddCodeList; bool fgAddCodeModf; - bool fgRngChkThrowAdded; AddCodeDsc* fgExcptnTargetCache[SCK_COUNT]; BasicBlock* fgRngChkTarget(BasicBlock* block, SpecialCodeKind kind); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 14dd9740c9963a..c776c86cbae804 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3777,7 +3777,6 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special tree = fgMorphArgs(tree); // Store the tree in the new basic block. - assert(!srcBlk->isEmpty()); if (!srcBlk->IsLIR()) { fgInsertStmtAtEnd(newBlk, fgNewStmtFromTree(tree)); From 63bcb91b8ed84e264e11113ba03ecb0435ee6495 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 13 Oct 2023 02:05:10 +0200 Subject: [PATCH 4/6] Remove fgRngChkThrowAdded --- src/coreclr/jit/fgbasic.cpp | 3 --- src/coreclr/jit/flowgraph.cpp | 1 - 2 files changed, 4 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 75078fc57f8d6f..643b57dabe7ac2 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -109,9 +109,6 @@ void Compiler::fgInit() /* This global flag is set whenever we remove a statement */ fgStmtRemoved = false; - /* This global flag is set whenever we add a throw block for a RngChk */ - fgRngChkThrowAdded = false; /* reset flag for fgIsCodeAdded() */ - /* Keep track of whether or not EH statements have been optimized */ fgOptimizedFinally = false; diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index c776c86cbae804..bdfda556789cc5 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3727,7 +3727,6 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special /* Remember that we're adding a new basic block */ fgAddCodeModf = true; - fgRngChkThrowAdded = true; /* Now figure out what code to insert */ From 0ec68ff7dbbbbe564753a4212ad3e96d5edce3fa Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 13 Oct 2023 04:13:34 +0200 Subject: [PATCH 5/6] Update flowgraph.cpp --- src/coreclr/jit/flowgraph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index bdfda556789cc5..7f81d609f40bdf 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3726,7 +3726,7 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special /* Remember that we're adding a new basic block */ - fgAddCodeModf = true; + fgAddCodeModf = true; /* Now figure out what code to insert */ From 19ec2c01cf1cb9061d31def655cce9c9d1e93bc6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 13 Oct 2023 13:47:28 +0200 Subject: [PATCH 6/6] Fix debug --- src/coreclr/jit/flowgraph.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 7f81d609f40bdf..27961bf5c35a0b 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3618,8 +3618,10 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special // arg slots on the stack frame if there are no other calls. compUsesThrowHelper = true; - if (!fgUseThrowHelperBlocks()) + if (!fgUseThrowHelperBlocks() && (kind != SCK_FAIL_FAST)) { + // We'll create a throw block in-place then (for better debugging) + // It's not needed for fail fast, since it's not recoverable anyway. return nullptr; } @@ -3797,7 +3799,7 @@ BasicBlock* Compiler::fgAddCodeRef(BasicBlock* srcBlk, unsigned refData, Special Compiler::AddCodeDsc* Compiler::fgFindExcptnTarget(SpecialCodeKind kind, unsigned refData) { - assert(fgUseThrowHelperBlocks()); + assert(fgUseThrowHelperBlocks() || (kind == SCK_FAIL_FAST)); if (!(fgExcptnTargetCache[kind] && // Try the cached value first fgExcptnTargetCache[kind]->acdData == refData)) {