From e2ba5791a65eb94fac2c05cf872a80bfebfe1627 Mon Sep 17 00:00:00 2001 From: Sebastian Nickolls Date: Fri, 24 Jan 2025 10:53:17 +0000 Subject: [PATCH 1/4] Improve compare-and-branch sequences produced by Emitter Introduces an overload of `genJumpToThrowHlpBlk` that allows you to pass a function that generates the branch code, before it creates an inline throw block. This allows us to choose compare-and-branch sequences such as `cbz` on ARM64 for checks added in the emitter, such as divide-by-zero. --- src/coreclr/jit/codegen.h | 50 +++++++++++++++++++++++++++++++ src/coreclr/jit/codegenarm64.cpp | 35 ++++++++++++++++++++-- src/coreclr/jit/codegencommon.cpp | 30 +++++++++++++++++++ 3 files changed, 112 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 36fe5e1f4b4e04..3555c470545df9 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -230,6 +230,54 @@ class CodeGen final : public CodeGenInterface void genExitCode(BasicBlock* block); + BasicBlock* genGetThrowHelper(SpecialCodeKind codeKind); + + // genEmitInlineThrow: Generate code for an inline exception. + void genEmitInlineThrow(SpecialCodeKind codeKind) + { + genEmitHelperCall(compiler->acdHelper(codeKind), 0, EA_UNKNOWN); + } + + // throwCodeFn callback follows concept -> void(*)(BasicBlock* target, bool isInline) + // + // For conditional jumps: + // If `isInline`, invert the condition for throw and fall into the exception block. + // Otherwise emit compare and jump with the normal throw condition. + // For unconditional jumps: + // Only emit the unconditional jump when `isInline == false`. + // When `isInline == true` the code will fallthrough to throw without any jump added. + // + // Parameter `target` gives a label to jump to, which is the throw block if + // `isInline == false`, else the continuation. + template + void genJumpToThrowHlpBlk(SpecialCodeKind codeKind, throwCodeFn emitJumpCode) + { + BasicBlock* target = genGetThrowHelper(codeKind); + if (target) + { + // check: + // if (checkPassed) + // goto throw; + // ... + // throw: + // throw(); + emitJumpCode(target, false); + } + else + { + // check: + // if (!checkPassed) + // goto continue; + // throw(); + // continue: + // ... + BasicBlock* over = genCreateTempLabel(); + emitJumpCode(over, true); + genEmitInlineThrow(codeKind); + genDefineTempLabel(over); + } + } + void genJumpToThrowHlpBlk(emitJumpKind jumpKind, SpecialCodeKind codeKind, BasicBlock* failBlk = nullptr); #if defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) @@ -1285,6 +1333,8 @@ class CodeGen final : public CodeGenInterface #endif #if defined(TARGET_ARM64) void genCodeForJumpCompare(GenTreeOpCC* tree); + void genCompareImmAndJump( + GenCondition::Code cond, regNumber reg, ssize_t compareImm, emitAttr size, BasicBlock* target); void genCodeForBfiz(GenTreeOp* tree); #endif // TARGET_ARM64 diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 8811349b8cdbb3..8d70e044665819 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -3621,9 +3621,10 @@ void CodeGen::genCodeForDivMod(GenTreeOp* tree) } else { - // Check if the divisor is zero throw a DivideByZeroException - emit->emitIns_R_I(INS_cmp, size, divisorReg, 0); - genJumpToThrowHlpBlk(EJ_eq, SCK_DIV_BY_ZERO); + genJumpToThrowHlpBlk(SCK_DIV_BY_ZERO, [&](BasicBlock* target, bool invert) { + GenCondition::Code cond = invert ? GenCondition::NE : GenCondition::EQ; + genCompareImmAndJump(cond, divisorReg, 0, size, target); + }); } } @@ -5064,6 +5065,34 @@ void CodeGen::genCodeForJumpCompare(GenTreeOpCC* tree) } } +void CodeGen::genCompareImmAndJump( + GenCondition::Code cond, regNumber reg, ssize_t compareImm, emitAttr size, BasicBlock* target) +{ + // For ARM64 we only expect equality comparisons. + assert((cond == GenCondition::EQ) || (cond == GenCondition::NE)); + + if (compareImm == 0) + { + // We can use cbz/cbnz + instruction ins = (cond == GenCondition::EQ) ? INS_cbz : INS_cbnz; + GetEmitter()->emitIns_J_R(ins, size, target, reg); + } + else if (isPow2(compareImm)) + { + // We can use tbz/tbnz + instruction ins = (cond == GenCondition::EQ) ? INS_tbz : INS_tbnz; + int imm = genLog2((size_t)compareImm); + GetEmitter()->emitIns_J_R_I(ins, size, target, reg, imm); + } + else + { + // Emit compare and branch pair default. + emitJumpKind jumpKind = (cond == GenCondition::EQ) ? EJ_eq : EJ_ne; + GetEmitter()->emitIns_R_I(INS_cmp, size, reg, compareImm); + inst_JMP(jumpKind, target); + } +} + //--------------------------------------------------------------------- // genSPtoFPdelta - return offset from the stack pointer (Initial-SP) to the frame pointer. The frame pointer // will point to the saved frame pointer slot (i.e., there will be frame pointer chaining). diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index d535dfb1454570..73c0b0f1f959bb 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1550,6 +1550,36 @@ void CodeGen::genExitCode(BasicBlock* block) genReserveEpilog(block); } +//--------------------------------------------------------------------------------- +// genGetThrowHelper: Search for the throw helper for the exception kind `codeKind` +BasicBlock* CodeGen::genGetThrowHelper(SpecialCodeKind codeKind) +{ + bool useThrowHlpBlk = compiler->fgUseThrowHelperBlocks(); +#if defined(UNIX_X86_ABI) + // TODO: Is this really UNIX_X86_ABI specific? Should we guard with compiler->UsesFunclets() instead? + // Inline exception-throwing code in funclet to make it possible to unwind funclet frames. + useThrowHlpBlk = useThrowHlpBlk && (compiler->funCurrentFunc()->funKind == FUNC_ROOT); +#endif // UNIX_X86_ABI + + BasicBlock* excpRaisingBlock = nullptr; + if (useThrowHlpBlk) + { + // For code with throw helper blocks, find and use the helper block for + // raising the exception. The block may be shared by other trees too. + Compiler::AddCodeDsc* add = compiler->fgFindExcptnTarget(codeKind, compiler->compCurBB); + PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block")); + assert(add->acdUsed); + excpRaisingBlock = add->acdDstBlk; +#if !FEATURE_FIXED_OUT_ARGS + assert(add->acdStkLvlInit || isFramePointerUsed()); +#endif // !FEATURE_FIXED_OUT_ARGS + + noway_assert(excpRaisingBlock != nullptr); + } + + return excpRaisingBlock; +} + //------------------------------------------------------------------------ // genJumpToThrowHlpBlk: Generate code for an out-of-line exception. // From ceb3763c063180cc5dc912c23a7848c7576a699a Mon Sep 17 00:00:00 2001 From: Sebastian Nickolls Date: Mon, 27 Jan 2025 13:28:37 +0000 Subject: [PATCH 2/4] Optimize bounds checks for zero index on ARM64 Emit `cbz` instead of `cmp+b.ls` when checking bounds for an access to the 0 index of an array. It can only throw when `arrayLength == 0`. Fixes #42514 --- src/coreclr/jit/codegen.h | 13 +++++++++---- src/coreclr/jit/codegenarmarch.cpp | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 3555c470545df9..28fa166cf973ee 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -250,10 +250,15 @@ class CodeGen final : public CodeGenInterface // Parameter `target` gives a label to jump to, which is the throw block if // `isInline == false`, else the continuation. template - void genJumpToThrowHlpBlk(SpecialCodeKind codeKind, throwCodeFn emitJumpCode) + void genJumpToThrowHlpBlk(SpecialCodeKind codeKind, throwCodeFn emitJumpCode, BasicBlock* throwBlock = nullptr) { - BasicBlock* target = genGetThrowHelper(codeKind); - if (target) + if (!throwBlock) + { + // If caller didn't supply a target block, then try to find a helper block. + throwBlock = genGetThrowHelper(codeKind); + } + + if (throwBlock) { // check: // if (checkPassed) @@ -261,7 +266,7 @@ class CodeGen final : public CodeGenInterface // ... // throw: // throw(); - emitJumpCode(target, false); + emitJumpCode(throwBlock, false); } else { diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index fb1e738f81451e..0d78cc5bbb32d0 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -1472,6 +1472,23 @@ void CodeGen::genRangeCheck(GenTree* oper) src1 = arrLen; src2 = arrIndex; jmpKind = EJ_ls; + +#if defined(TARGET_ARM64) + if (arrIndex->IsIntegralConst(0)) + { + assert(!arrLen->isContained()); + // For (index == 0), we can just test if (length == 0) as this is the only case that would throw. + // This may lead to an optimization by using cbz/tbnz. + genJumpToThrowHlpBlk( + bndsChk->gtThrowKind, + [&](BasicBlock* target, bool isInline) { + genCompareImmAndJump(isInline ? GenCondition::NE : GenCondition::EQ, arrLen->GetReg(), 0, + emitActualTypeSize(arrLen), target); + }, + bndsChk->gtIndRngFailBB); + return; + } +#endif } else { From 32334e23019ed1e36bc872ee52d81af0c9089c66 Mon Sep 17 00:00:00 2001 From: Sebastian Nickolls Date: Thu, 30 Jan 2025 09:55:51 +0000 Subject: [PATCH 3/4] Move `genGetThrowHelper` to codegenarm64.cpp --- src/coreclr/jit/codegen.h | 2 ++ src/coreclr/jit/codegenarm64.cpp | 23 +++++++++++++++++++++++ src/coreclr/jit/codegencommon.cpp | 30 ------------------------------ 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 28fa166cf973ee..b8f4d99353208a 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -230,6 +230,7 @@ class CodeGen final : public CodeGenInterface void genExitCode(BasicBlock* block); +#if defined(TARGET_ARM64) BasicBlock* genGetThrowHelper(SpecialCodeKind codeKind); // genEmitInlineThrow: Generate code for an inline exception. @@ -282,6 +283,7 @@ class CodeGen final : public CodeGenInterface genDefineTempLabel(over); } } +#endif void genJumpToThrowHlpBlk(emitJumpKind jumpKind, SpecialCodeKind codeKind, BasicBlock* failBlk = nullptr); diff --git a/src/coreclr/jit/codegenarm64.cpp b/src/coreclr/jit/codegenarm64.cpp index 8d70e044665819..b8308139c79dc5 100644 --- a/src/coreclr/jit/codegenarm64.cpp +++ b/src/coreclr/jit/codegenarm64.cpp @@ -5961,4 +5961,27 @@ insOpts CodeGen::ShiftOpToInsOpts(genTreeOps shiftOp) } } +//--------------------------------------------------------------------------------- +// genGetThrowHelper: Search for the throw helper for the exception kind `codeKind` +BasicBlock* CodeGen::genGetThrowHelper(SpecialCodeKind codeKind) +{ + BasicBlock* excpRaisingBlock = nullptr; + if (compiler->fgUseThrowHelperBlocks()) + { + // For code with throw helper blocks, find and use the helper block for + // raising the exception. The block may be shared by other trees too. + Compiler::AddCodeDsc* add = compiler->fgFindExcptnTarget(codeKind, compiler->compCurBB); + PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block")); + assert(add->acdUsed); + excpRaisingBlock = add->acdDstBlk; +#if !FEATURE_FIXED_OUT_ARGS + assert(add->acdStkLvlInit || isFramePointerUsed()); +#endif // !FEATURE_FIXED_OUT_ARGS + + noway_assert(excpRaisingBlock != nullptr); + } + + return excpRaisingBlock; +} + #endif // TARGET_ARM64 diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 73c0b0f1f959bb..d535dfb1454570 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1550,36 +1550,6 @@ void CodeGen::genExitCode(BasicBlock* block) genReserveEpilog(block); } -//--------------------------------------------------------------------------------- -// genGetThrowHelper: Search for the throw helper for the exception kind `codeKind` -BasicBlock* CodeGen::genGetThrowHelper(SpecialCodeKind codeKind) -{ - bool useThrowHlpBlk = compiler->fgUseThrowHelperBlocks(); -#if defined(UNIX_X86_ABI) - // TODO: Is this really UNIX_X86_ABI specific? Should we guard with compiler->UsesFunclets() instead? - // Inline exception-throwing code in funclet to make it possible to unwind funclet frames. - useThrowHlpBlk = useThrowHlpBlk && (compiler->funCurrentFunc()->funKind == FUNC_ROOT); -#endif // UNIX_X86_ABI - - BasicBlock* excpRaisingBlock = nullptr; - if (useThrowHlpBlk) - { - // For code with throw helper blocks, find and use the helper block for - // raising the exception. The block may be shared by other trees too. - Compiler::AddCodeDsc* add = compiler->fgFindExcptnTarget(codeKind, compiler->compCurBB); - PREFIX_ASSUME_MSG((add != nullptr), ("ERROR: failed to find exception throw block")); - assert(add->acdUsed); - excpRaisingBlock = add->acdDstBlk; -#if !FEATURE_FIXED_OUT_ARGS - assert(add->acdStkLvlInit || isFramePointerUsed()); -#endif // !FEATURE_FIXED_OUT_ARGS - - noway_assert(excpRaisingBlock != nullptr); - } - - return excpRaisingBlock; -} - //------------------------------------------------------------------------ // genJumpToThrowHlpBlk: Generate code for an out-of-line exception. // From 10e79291c0d371941febeab7621a7a06ad0727a2 Mon Sep 17 00:00:00 2001 From: Sebastian Nickolls Date: Thu, 30 Jan 2025 11:09:29 +0000 Subject: [PATCH 4/4] Use `GetRegNum` instead of `GetReg` --- src/coreclr/jit/codegenarmarch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/codegenarmarch.cpp b/src/coreclr/jit/codegenarmarch.cpp index 0d78cc5bbb32d0..5b098dae76b908 100644 --- a/src/coreclr/jit/codegenarmarch.cpp +++ b/src/coreclr/jit/codegenarmarch.cpp @@ -1482,7 +1482,7 @@ void CodeGen::genRangeCheck(GenTree* oper) genJumpToThrowHlpBlk( bndsChk->gtThrowKind, [&](BasicBlock* target, bool isInline) { - genCompareImmAndJump(isInline ? GenCondition::NE : GenCondition::EQ, arrLen->GetReg(), 0, + genCompareImmAndJump(isInline ? GenCondition::NE : GenCondition::EQ, arrLen->GetRegNum(), 0, emitActualTypeSize(arrLen), target); }, bndsChk->gtIndRngFailBB);