From 4603b430f16b493fbc68fc3486dc821e94d45688 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 25 Jun 2026 07:52:34 -0700 Subject: [PATCH 1/2] JIT: ensure magic divisors are always marked DONT_CSE for all targets fgMorphSmpOpOptional was calling CheckDivideByConstOptimized for GT_UDIV/GT_UMOD but not for signed GT_DIV, leaving the constant divisor unmarked. If CSE later rewrites it to a LCL_VAR, magic-divide expansion in LowerSignedDivOrMod bails and falls back to idiv. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/morph.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8f32ee00f0d34c..457373e3f26830 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -11396,6 +11396,7 @@ GenTree* Compiler::fgMorphSmpOpOptional(GenTreeOp* tree, bool* optAssertionPropD DEBUG_DESTROY_NODE(tree); return op1; } + tree->AsOp()->CheckDivideByConstOptimized(this); break; case GT_UDIV: From f11b2faf61a2eb52b50118eebdeecb8643dd0770 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 25 Jun 2026 08:51:04 -0700 Subject: [PATCH 2/2] JIT: extend UsesDivideByConstOptimized to negative magic and pow2 divisors Lowering's TryLowerConstIntDivOrMod generates magic-divide for any non-pow2 signed divisor with |d| >= 3 and a shift+neg sequence for negative pow2 divisors. Mirror that here so the corresponding DONT_CSE marker is set on the divisor constant. Addresses review feedback on dotnet/runtime#129856. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/gentree.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index ead7edca7308b0..f793bf9cc8259e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -10464,7 +10464,12 @@ bool GenTreeOp::UsesDivideByConstOptimized(Compiler* comp) // x / -1 can't be optimized because INT_MIN / -1 is required to throw an exception. return false; } - else if (isPow2(divisorValue)) + // Match the lowering acceptance for absolute value: lowering's TryLowerConstIntDivOrMod + // turns negative pow2 divisors into shift+neg and negative non-pow2 divisors into + // signed magic-divide sequences, both of which require the divisor be a literal. + size_t absDivisorValue = (divisorValue == SSIZE_T_MIN) ? static_cast(divisorValue) + : static_cast(std::abs(divisorValue)); + if (isPow2(absDivisorValue)) { return true; } @@ -10514,7 +10519,9 @@ bool GenTreeOp::UsesDivideByConstOptimized(Compiler* comp) // TODO-ARM-CQ: Currently there's no GT_MULHI for ARM32 #if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) - if (!comp->opts.MinOpts() && ((divisorValue >= 3) || !isSignedDivide)) + // For signed divides also accept negative magic divisors (e.g. -3, -5); lowering generates + // a reciprocal-multiply sequence for those, see TryLowerConstIntDivOrMod. + if (!comp->opts.MinOpts() && (!isSignedDivide || (divisorValue >= 3) || (divisorValue <= -3))) { // All checks pass we can perform the division operation using a reciprocal multiply. return true;