From 8e52193374351b03899d509c7b0a246cda39eec7 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 12 Jun 2026 15:30:49 +0200 Subject: [PATCH 1/4] More cleanups in rangecheck/assertionprop --- src/coreclr/inc/switches.h | 4 -- src/coreclr/jit/assertionprop.cpp | 64 +++++-------------------------- src/coreclr/jit/compiler.h | 2 - src/coreclr/jit/gentree.h | 2 - src/coreclr/jit/jitconfigvalues.h | 4 -- src/coreclr/jit/optimizer.cpp | 20 ---------- 6 files changed, 10 insertions(+), 86 deletions(-) diff --git a/src/coreclr/inc/switches.h b/src/coreclr/inc/switches.h index 38e33513516be2..74e2f8b28004c5 100644 --- a/src/coreclr/inc/switches.h +++ b/src/coreclr/inc/switches.h @@ -119,10 +119,6 @@ #endif // _DEBUG -// MUST NEVER CHECK IN WITH THIS ENABLED. -// This is just for convenience in doing performance investigations in a checked-out enlistment. -// #define FEATURE_ENABLE_NO_RANGE_CHECKS - // This controls whether a compilation-timing feature that relies on Windows APIs, if available, else direct // hardware instructions (rdtsc), for accessing high-resolution hardware timers is enabled. This is disabled // in Silverlight (just to avoid thinking about whether the extra code space is worthwhile). diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 818d18dff427f0..2948e402cfa543 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -4958,23 +4958,6 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, return nullptr; } -/***************************************************************************** - * - * Given a tree with an array bounds check node, eliminate it because it was - * checked already in the program. - */ -GenTree* Compiler::optAssertionProp_Comma(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt) -{ - // Remove the bounds check as part of the GT_COMMA node since we need parent pointer to remove nodes. - // When processing visits the bounds check, it sets the throw kind to None if the check is redundant. - if (tree->gtGetOp1()->OperIs(GT_BOUNDS_CHECK) && ((tree->gtGetOp1()->gtFlags & GTF_CHK_INDEX_INBND) != 0)) - { - optRemoveCommaBasedRangeCheck(tree, stmt); - return optAssertionProp_Update(tree, tree, stmt); - } - return nullptr; -} - //------------------------------------------------------------------------ // optAssertionProp_Ind: see if we can prove the indirection can't cause // and exception. @@ -5458,59 +5441,34 @@ GenTree* Compiler::optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCal */ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt) { + assert(tree->OperIs(GT_BOUNDS_CHECK)); if (optLocalAssertionProp) { + // We don't have the right kind of assertions to optimize bounds checks in local assertion prop. return nullptr; } - assert(tree->OperIs(GT_BOUNDS_CHECK)); - -#ifdef FEATURE_ENABLE_NO_RANGE_CHECKS - if (JitConfig.JitNoRngChks()) - { -#ifdef DEBUG - if (verbose) - { - printf("\nFlagging check redundant due to JitNoRngChks in " FMT_BB ":\n", compCurBB->bbNum); - gtDispTree(tree, nullptr, nullptr, true); - } -#endif // DEBUG - tree->gtFlags |= GTF_CHK_INDEX_INBND; - return nullptr; - } -#endif // FEATURE_ENABLE_NO_RANGE_CHECKS - GenTreeBoundsChk* arrBndsChk = tree->AsBoundsChk(); GenTree* arrBndsChkIdx = arrBndsChk->GetIndex(); GenTree* arrBndsChkLen = arrBndsChk->GetArrayLength(); - ValueNum vnCurIdx = vnStore->VNConservativeNormalValue(arrBndsChkIdx->gtVNPair); - ValueNum vnCurLen = vnStore->VNConservativeNormalValue(arrBndsChkLen->gtVNPair); + ValueNum vnCurIdx = optConservativeNormalVN(arrBndsChkIdx); + ValueNum vnCurLen = optConservativeNormalVN(arrBndsChkLen); auto dropBoundsCheck = [&](INDEBUG(const char* reason)) -> GenTree* { - JITDUMP("\nVN based redundant (%s) bounds check assertion prop in " FMT_BB ":\n", reason, compCurBB->bbNum); + JITDUMP("\nRemoving redundant (%s) bounds check in " FMT_BB ":\n", reason, compCurBB->bbNum); DISPTREE(tree); - if (arrBndsChk != stmt->GetRootNode()) - { - // Defer the removal. - arrBndsChk->gtFlags |= GTF_CHK_INDEX_INBND; - return nullptr; - } - GenTree* newTree = optRemoveStandaloneRangeCheck(arrBndsChk, stmt); - return optAssertionProp_Update(newTree, arrBndsChk, stmt); + // Extract the side-effects of idx and len without taking the root (GT_BOUNDS_CHECK itself) into account. + GenTree* nothing = gtWrapWithSideEffects(gtNewNothingNode(), tree, GTF_ALL_EFFECT, /*ignoreRoot*/ true); + return optAssertionProp_Update(nothing, arrBndsChk, stmt); }; BitVecOps::Iter iter(apTraits, assertions); unsigned index = 0; while (iter.NextElem(&index)) { - AssertionIndex assertionIndex = GetAssertionIndex(index); - if (assertionIndex > optAssertionCount) - { - break; - } // If it is not a nothrow assertion, skip. - const AssertionDsc& curAssertion = optGetAssertion(assertionIndex); + const AssertionDsc& curAssertion = optGetAssertion(GetAssertionIndex(index)); if (!curAssertion.IsBoundsCheckNoThrow()) { continue; @@ -5652,6 +5610,7 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree return dropBoundsCheck(INDEBUG("upper bound of index is less than lower bound of length")); } } + return nullptr; } @@ -5800,9 +5759,6 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree, case GT_BOUNDS_CHECK: return optAssertionProp_BndsChk(assertions, tree, stmt); - case GT_COMMA: - return optAssertionProp_Comma(assertions, tree, stmt); - case GT_CAST: return optAssertionProp_Cast(assertions, tree->AsCast(), stmt, block); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index de116a31f9ebb1..39ea5d07b67e98 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7427,7 +7427,6 @@ class Compiler public: PhaseStatus rangeCheckPhase(); GenTree* optRemoveRangeCheck(GenTreeBoundsChk* check, GenTree* comma, Statement* stmt); - GenTree* optRemoveStandaloneRangeCheck(GenTreeBoundsChk* check, Statement* stmt); void optRemoveCommaBasedRangeCheck(GenTree* comma, Statement* stmt); protected: @@ -9134,7 +9133,6 @@ class Compiler GenTree* optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCast* cast, Statement* stmt, BasicBlock* block); GenTree* optAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCall* call, Statement* stmt); GenTree* optAssertionProp_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt, BasicBlock* block); - GenTree* optAssertionProp_Comma(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); GenTree* optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree* tree, Statement* stmt); GenTree* optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions, GenTree* tree, diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 9af563110a595f..fd6373b9000774 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -528,8 +528,6 @@ enum GenTreeFlags : unsigned GTF_DIV_MOD_NO_OVERFLOW = 0x40000000, // GT_DIV, GT_MOD -- Div or mod definitely does not overflow. - GTF_CHK_INDEX_INBND = 0x80000000, // GT_BOUNDS_CHECK -- have proven this check is always in-bounds - GTF_ARRLEN_NONFAULTING = 0x20000000, // GT_ARR_LENGTH -- An array length operation that cannot fault. Same as GT_IND_NONFAULTING. GTF_MDARRLEN_NONFAULTING = 0x20000000, // GT_MDARR_LENGTH -- An MD array length operation that cannot fault. Same as GT_IND_NONFAULTING. diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 561191bd4411a5..afccec6c542074 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -574,10 +574,6 @@ RELEASE_CONFIG_INTEGER(JitInlineSIMDMultiplier, "JitInlineSIMDMultiplier", 3) // Ex lclMAX_TRACKED constant. RELEASE_CONFIG_INTEGER(JitMaxLocalsToTrack, "JitMaxLocalsToTrack", 0x400) -#if defined(FEATURE_ENABLE_NO_RANGE_CHECKS) -RELEASE_CONFIG_INTEGER(JitNoRngChks, "JitNoRngChks", 0) // If 1, don't generate range checks -#endif - OPT_CONFIG_INTEGER(JitDoAssertionProp, "JitDoAssertionProp", 1) // Perform assertion propagation optimization OPT_CONFIG_INTEGER(JitDoCopyProp, "JitDoCopyProp", 1) // Perform copy propagation on variables that appear redundant OPT_CONFIG_INTEGER(JitDoOptimizeIVs, "JitDoOptimizeIVs", 1) // Perform optimization of induction variables diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index a1ccf0717a9354..3fe6c0a68469f9 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -5593,26 +5593,6 @@ GenTree* Compiler::optRemoveRangeCheck(GenTreeBoundsChk* check, GenTree* comma, return check; } -//------------------------------------------------------------------------------ -// optRemoveStandaloneRangeCheck : A thin wrapper over optRemoveRangeCheck that removes standalone checks. -// -// Arguments: -// check - The standalone top-level CHECK node. -// stmt - The statement "check" is a root node of. -// -// Return Value: -// If "check" has no side effects, it is retuned, bashed to a no-op. -// If it has side effects, the tree that executes them is returned. -// -GenTree* Compiler::optRemoveStandaloneRangeCheck(GenTreeBoundsChk* check, Statement* stmt) -{ - assert(check != nullptr); - assert(stmt != nullptr); - assert(check == stmt->GetRootNode()); - - return optRemoveRangeCheck(check, nullptr, stmt); -} - //------------------------------------------------------------------------------ // optRemoveCommaBasedRangeCheck : A thin wrapper over optRemoveRangeCheck that removes COMMA-based checks. // From bf4100381fcac559019d71561962f99af7c841b8 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 12 Jun 2026 17:50:51 +0200 Subject: [PATCH 2/4] fix regressions --- src/coreclr/jit/assertionprop.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 2948e402cfa543..864aa443ccbc3a 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -5740,6 +5740,8 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree, return optAssertionProp_ModDiv(assertions, tree->AsOp(), stmt, block); case GT_ARR_LENGTH: + case GT_MDARR_LENGTH: + case GT_MDARR_LOWER_BOUND: // Unfortunately, doing this in LocalAP produces an asymmetry in exception sets between // uses/defs that CSE does not manage to make good use of. As a result, some bounds checks are no longer // removed. From 2ea73afd69e33f4ed238afe5eecccc9f5cf3f858 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 12 Jun 2026 21:38:22 +0200 Subject: [PATCH 3/4] test 2 --- src/coreclr/jit/assertionprop.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 864aa443ccbc3a..a63cd0eb78a0e3 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -5739,9 +5739,16 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree, case GT_UDIV: return optAssertionProp_ModDiv(assertions, tree->AsOp(), stmt, block); - case GT_ARR_LENGTH: +#if !defined(TARGET_AMD64) + // An MD array access loads several metadata fields (the per-dimension lengths and lower bounds), + // most of which are needed for the element-address computation, not just for the bounds checks. + // Proving them non-faulting pins each load with an ordering side effect (see + // On targets with load-pair addressing (e.g. arm64) GTF_ORDER_SIDEEFFCT + // prevents these adjacent metadata loads from being combined into ldp. case GT_MDARR_LENGTH: case GT_MDARR_LOWER_BOUND: +#endif + case GT_ARR_LENGTH: // Unfortunately, doing this in LocalAP produces an asymmetry in exception sets between // uses/defs that CSE does not manage to make good use of. As a result, some bounds checks are no longer // removed. From 46524143b0fc5610a26c7d0d0594ca7f17600f53 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 12 Jun 2026 22:20:26 +0200 Subject: [PATCH 4/4] Update assertionprop.cpp --- src/coreclr/jit/assertionprop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index a63cd0eb78a0e3..3f71da3efbdaf0 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -5739,7 +5739,7 @@ GenTree* Compiler::optAssertionProp(ASSERT_VALARG_TP assertions, GenTree* tree, case GT_UDIV: return optAssertionProp_ModDiv(assertions, tree->AsOp(), stmt, block); -#if !defined(TARGET_AMD64) +#if defined(TARGET_AMD64) // An MD array access loads several metadata fields (the per-dimension lengths and lower bounds), // most of which are needed for the element-address computation, not just for the bounds checks. // Proving them non-faulting pins each load with an ordering side effect (see