Skip to content

Commit 0b9d2cc

Browse files
EgorBoCopilotteo-tsirpanis
authored
Remove bounds checks for Phi indices (#121640)
Closes #121617 In global assertion prop we have an optimization that tries to optimize bound checks for `arr[cns]` if we already have ArrBnds assertion for the same array with another constant index that is greater than this one, e.g.: ``` arr[2] = 0; // creates an assertion that arr is at least 3 elements. arr[1] = 0; // bound check is optimized away ``` This PR extends this logic by handling Phi indices. Should remove bounds checks for cases like: ```cs arr[cond1 ? 10 : 12] = 0; // means arr has at least 11 elements arr[8] = 0; // redundant bounds check ``` or even: ```cs arr[cond1 ? 10 : 12] = 0; arr[cond2 ? 1 : 2] = 0; ``` Some nice [diffs](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1203834&view=ms.vss-build-web.run-extensions-tab) (although, mostly because I removed unnecessary TypeOfVN checks) --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Theodore Tsirpanis <[email protected]>
1 parent e91f40d commit 0b9d2cc

File tree

1 file changed

+30
-13
lines changed

1 file changed

+30
-13
lines changed

src/coreclr/jit/assertionprop.cpp

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5437,21 +5437,38 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree
54375437
{
54385438
return dropBoundsCheck(INDEBUG("a[*] followed by a[0]"));
54395439
}
5440-
// Do we have two constant indexes?
5441-
else if (vnStore->IsVNConstant(curAssertion->op1.bnd.vnIdx) && vnStore->IsVNConstant(vnCurIdx))
5440+
else
54425441
{
5443-
// Make sure the types match.
5444-
var_types type1 = vnStore->TypeOfVN(curAssertion->op1.bnd.vnIdx);
5445-
var_types type2 = vnStore->TypeOfVN(vnCurIdx);
5446-
5447-
if (type1 == type2 && type1 == TYP_INT)
5442+
// index1 doesn't have to be a constant, it can be a Phi each predecessor of which is a constant.
5443+
// The smallest of those is what we can rely on. Example:
5444+
//
5445+
// arr[cond ? 10 : 5] = 0; // arr is at least 6 elements long
5446+
// arr[2] = 0; // arr must be at least 3 elements long
5447+
//
5448+
// or even:
5449+
//
5450+
// arr[cond ? 10 : 5] = 0; // arr is at least 6 elements long
5451+
// arr[cond ? 1 : 2] = 0; // arr must be at least 3 elements long
5452+
//
5453+
auto tryGetMaxOrMinConst = [this](ValueNum vn, bool getMin, int* index) -> bool {
5454+
*index = getMin ? INT_MAX : INT_MIN;
5455+
return vnStore->VNVisitReachingVNs(vn,
5456+
[this, index, getMin](ValueNum vn) -> ValueNumStore::VNVisit {
5457+
int cns = 0;
5458+
if (vnStore->IsVNIntegralConstant(vn, &cns))
5459+
{
5460+
*index = getMin ? min(*index, cns) : max(*index, cns);
5461+
return ValueNumStore::VNVisit::Continue;
5462+
}
5463+
return ValueNumStore::VNVisit::Abort;
5464+
}) == ValueNumStore::VNVisit::Continue;
5465+
};
5466+
5467+
int index1;
5468+
int index2;
5469+
if (tryGetMaxOrMinConst(curAssertion->op1.bnd.vnIdx, /*min*/ true, &index1) &&
5470+
tryGetMaxOrMinConst(vnCurIdx, /*max*/ false, &index2))
54485471
{
5449-
int index1 = vnStore->ConstantValue<int>(curAssertion->op1.bnd.vnIdx);
5450-
int index2 = vnStore->ConstantValue<int>(vnCurIdx);
5451-
5452-
// the case where index1 == index2 should have been handled above
5453-
assert(index1 != index2);
5454-
54555472
// It can always be considered as redundant with any previous higher constant value
54565473
// a[K1] followed by a[K2], with K2 >= 0 and K1 >= K2
54575474
if (index2 >= 0 && index1 >= index2)

0 commit comments

Comments
 (0)