Repository: dotnet/runtime
File: src/coreclr/jit/assertionprop.cpp (C++)

Code before:
            if (curAssertion->op1.bnd.vnIdx == vnCurIdx)
            {
                return dropBoundsCheck(INDEBUG("a[i] followed by a[i]"));
            }
            // Are we using zero as the index?
            // It can always be considered as redundant with any previous value
            //       a[*] followed by a[0]
            else if (vnCurIdx == vnStore->VNZeroForType(arrBndsChk->GetIndex()->TypeGet()))
            {
                return dropBoundsCheck(INDEBUG("a[*] followed by a[0]"));
            }
            else
            {
                // index1 doesn't have to be a constant, it can be a Phi each predecessor of which is a constant.
                // The smallest of those is what we can rely on. Example:
                //
                //  arr[cond ? 10 : 5] = 0;  // arr is at least 6 elements long
                //  arr[2] = 0;              // arr must be at least 3 elements long
                //
                // or even:
                //
                //  arr[cond ? 10 : 5] = 0; // arr is at least 6 elements long
                //  arr[cond ? 1 : 2] = 0;  // arr must be at least 3 elements long
                //
                auto tryGetMaxOrMinConst = [this](ValueNum vn, bool getMin, int* index) -> bool {
                    *index = INT_MAX;
                    return vnStore->VNVisitReachingVNs(vn,
                                                       [this, &index, &getMin](ValueNum vn) -> ValueNumStore::VNVisit {
                        int cns = 0;
                        if (vnStore->IsVNIntegralConstant(vn, &cns))
                        {
                            *index = getMin ? max(*index, cns) : min(*index, cns);
                            return ValueNumStore::VNVisit::Continue;
                        }
                        return ValueNumStore::VNVisit::Abort;
                    }) == ValueNumStore::VNVisit::Continue;
                };

                int index1;
                int index2;
                if (tryGetMaxOrMinConst(curAssertion->op1.bnd.vnIdx, /*min*/ true, &index1) &&
                    tryGetMaxOrMinConst(vnCurIdx, /*max*/ false, &index2))
                {
                    // It can always be considered as redundant with any previous higher constant value
                    //       a[K1] followed by a[K2], with K2 >= 0 and K1 >= K2
                    if (index2 >= 0 && index1 >= index2)
                    {
                        return dropBoundsCheck(INDEBUG("a[K1] followed by a[K2], with K2 >= 0 and K1 >= K2"));
                    }
                }
            }

Review comment (bug):
The initialization value is incorrect for the maximum case. When `getMin` is false (getting maximum), `*index` should be initialized to `INT_MIN`, not `INT_MAX`. With the current code, when finding the maximum value across phi predecessors, `INT_MAX` will always be larger than any actual value, resulting in an incorrect maximum of `INT_MAX`.
```suggestion
                    *index = getMin ? INT_MAX : INT_MIN;
```

Code after:
            if (curAssertion->op1.bnd.vnIdx == vnCurIdx)
            {
                return dropBoundsCheck(INDEBUG("a[i] followed by a[i]"));
            }
            // Are we using zero as the index?
            // It can always be considered as redundant with any previous value
            //       a[*] followed by a[0]
            else if (vnCurIdx == vnStore->VNZeroForType(arrBndsChk->GetIndex()->TypeGet()))
            {
                return dropBoundsCheck(INDEBUG("a[*] followed by a[0]"));
            }
            else
            {
                // index1 doesn't have to be a constant, it can be a Phi each predecessor of which is a constant.
                // The smallest of those is what we can rely on. Example:
                //
                //  arr[cond ? 10 : 5] = 0;  // arr is at least 6 elements long
                //  arr[2] = 0;              // arr must be at least 3 elements long
                //
                // or even:
                //
                //  arr[cond ? 10 : 5] = 0; // arr is at least 6 elements long
                //  arr[cond ? 1 : 2] = 0;  // arr must be at least 3 elements long
                //
                auto tryGetMaxOrMinConst = [this](ValueNum vn, bool getMin, int* index) -> bool {
                    *index = getMin ? INT_MAX : INT_MIN;
                    return vnStore->VNVisitReachingVNs(vn,
                                                       [this, index, getMin](ValueNum vn) -> ValueNumStore::VNVisit {
                        int cns = 0;
                        if (vnStore->IsVNIntegralConstant(vn, &cns))
                        {
                            *index = getMin ? min(*index, cns) : max(*index, cns);
                            return ValueNumStore::VNVisit::Continue;
                        }
                        return ValueNumStore::VNVisit::Abort;
                    }) == ValueNumStore::VNVisit::Continue;
                };

                int index1;
                int index2;
                if (tryGetMaxOrMinConst(curAssertion->op1.bnd.vnIdx, /*min*/ true, &index1) &&
                    tryGetMaxOrMinConst(vnCurIdx, /*max*/ false, &index2))
                {
                    // It can always be considered as redundant with any previous higher constant value
                    //       a[K1] followed by a[K2], with K2 >= 0 and K1 >= K2
                    if (index2 >= 0 && index1 >= index2)
                    {
                        return dropBoundsCheck(INDEBUG("a[K1] followed by a[K2], with K2 >= 0 and K1 >= K2"));
                    }
                }
            }