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

Code before:
            //       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"));
                    }
                }
            }
            // Extend this to remove additional redundant bounds checks:
            // i.e.  a[i+1] followed by a[i]  by using the VN(i+1) >= VN(i)
            //       a[i]   followed by a[j]  when j is known to be >= i
            //       a[i]   followed by a[5]  when i is known to be >= 5
        }
    }

Review comment (suggestion):
The min/max logic is inverted. When `getMin` is true, we want the minimum value, so we should use `min(*index, cns)`, not `max(*index, cns)`. Similarly, when `getMin` is false (getting maximum), we should use `max(*index, cns)`, not `min(*index, cns)`. The correct logic should be: `*index = getMin ? min(*index, cns) : max(*index, cns);`
```suggestion
                            *index = getMin ? min(*index, cns) : max(*index, cns);
```

Code after:
            //       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"));
                    }
                }
            }
            // Extend this to remove additional redundant bounds checks:
            // i.e.  a[i+1] followed by a[i]  by using the VN(i+1) >= VN(i)
            //       a[i]   followed by a[j]  when j is known to be >= i
            //       a[i]   followed by a[5]  when i is known to be >= 5
        }
    }