Reapply "Use rangecheck in assertprop (dotnet#112766)" (dotnet#112872)

EgorBo · EgorBo · commit 76e603226d8d · 2025-02-25T14:04:49.000+01:00
This reverts commit db681fb.
diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
@@ -11,6 +11,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 */
 
 #include "jitpch.h"
+#include "rangecheck.h"
 #ifdef _MSC_VER
 #pragma hdrstop
 #endif
@@ -4122,6 +4123,69 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions,
             }
         }
     }
+
+    if (*isKnownNonZero && *isKnownNonNegative)
+    {
+        return;
+    }
+
+    // Let's see if MergeEdgeAssertions can help us:
+    if (tree->TypeIs(TYP_INT))
+    {
+        // See if (X + CNS) is known to be non-negative
+        if (tree->OperIs(GT_ADD) && tree->gtGetOp2()->IsIntCnsFitsInI32())
+        {
+            Range    rng = Range(Limit(Limit::keDependent));
+            ValueNum vn  = vnStore->VNConservativeNormalValue(tree->gtGetOp1()->gtVNPair);
+            RangeCheck::MergeEdgeAssertions(this, vn, ValueNumStore::NoVN, assertions, &rng, false);
+
+            int cns = static_cast<int>(tree->gtGetOp2()->AsIntCon()->IconValue());
+            rng.LowerLimit().AddConstant(cns);
+
+            if ((rng.LowerLimit().IsConstant() && !rng.LowerLimit().AddConstant(cns)) ||
+                (rng.UpperLimit().IsConstant() && !rng.UpperLimit().AddConstant(cns)))
+            {
+                // Add cns to both bounds if they are constants. Make sure the addition doesn't overflow.
+                return;
+            }
+
+            if (rng.LowerLimit().IsConstant())
+            {
+                // E.g. "X + -8" when X's range is [8..unknown]
+                // it's safe to say "X + -8" is non-negative
+                if ((rng.LowerLimit().GetConstant() == 0))
+                {
+                    *isKnownNonNegative = true;
+                }
+
+                // E.g. "X + 8" when X's range is [0..CNS]
+                // Here we have to check the upper bound as well to avoid overflow
+                if ((rng.LowerLimit().GetConstant() > 0) && rng.UpperLimit().IsConstant() &&
+                    rng.UpperLimit().GetConstant() > rng.LowerLimit().GetConstant())
+                {
+                    *isKnownNonNegative = true;
+                    *isKnownNonZero     = true;
+                }
+            }
+        }
+        else
+        {
+            Range rng = Range(Limit(Limit::keDependent));
+            RangeCheck::MergeEdgeAssertions(this, treeVN, ValueNumStore::NoVN, assertions, &rng, false);
+            Limit lowerBound = rng.LowerLimit();
+            if (lowerBound.IsConstant())
+            {
+                if (lowerBound.GetConstant() >= 0)
+                {
+                    *isKnownNonNegative = true;
+                }
+                if (lowerBound.GetConstant() > 0)
+                {
+                    *isKnownNonZero = true;
+                }
+            }
+        }
+    }
 }
 
 //------------------------------------------------------------------------
@@ -4848,12 +4912,6 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCas
     // Skip over a GT_COMMA node(s), if necessary to get to the lcl.
     GenTree* lcl = op1->gtEffectiveVal();
 
-    // If we don't have a cast of a LCL_VAR then bail.
-    if (!lcl->OperIs(GT_LCL_VAR))
-    {
-        return nullptr;
-    }
-
     // Try and see if we can make this cast into a cheaper zero-extending version
     // if the input is known to be non-negative.
     if (!cast->IsUnsigned() && genActualTypeIsInt(lcl) && cast->TypeIs(TYP_LONG) && (TARGET_POINTER_SIZE == 8))
@@ -4867,6 +4925,12 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCas
         }
     }
 
+    // If we don't have a cast of a LCL_VAR then bail.
+    if (!lcl->OperIs(GT_LCL_VAR))
+    {
+        return nullptr;
+    }
+
     IntegralRange  range = IntegralRange::ForCastInput(cast);
     AssertionIndex index = optAssertionIsSubrange(lcl, range, assertions);
     if (index != NO_ASSERTION_INDEX)
diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp
@@ -251,7 +251,7 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree*
         {
             JITDUMP("Looking for array size assertions for: " FMT_VN "\n", arrLenVn);
             Range arrLength = Range(Limit(Limit::keDependent));
-            MergeEdgeAssertions(arrLenVn, block->bbAssertionIn, &arrLength);
+            MergeEdgeAssertions(m_pCompiler, arrLenVn, arrLenVn, block->bbAssertionIn, &arrLength);
             if (arrLength.lLimit.IsConstant())
             {
                 arrSize = arrLength.lLimit.GetConstant();
@@ -640,20 +640,28 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
 
     LclSsaVarDsc* ssaData     = m_pCompiler->lvaGetDesc(lcl)->GetPerSsaData(lcl->GetSsaNum());
     ValueNum      normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair);
-    MergeEdgeAssertions(normalLclVN, assertions, pRange);
+    ValueNum      arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->GetArrayLength()->gtVNPair);
+    MergeEdgeAssertions(m_pCompiler, normalLclVN, arrLenVN, assertions, pRange);
 }
 
 //------------------------------------------------------------------------
 // MergeEdgeAssertions: Merge assertions on the edge flowing into the block about a variable
 //
 // Arguments:
-//    normalLclVN - the value number to look for assertions for
-//    assertions - the assertions to use
-//    pRange - the range to tighten with assertions
+//    comp             - the compiler instance
+//    normalLclVN      - the value number to look for assertions for
+//    preferredBoundVN - when this VN is set, it will be given preference over constant limits
+//    assertions       - the assertions to use
+//    pRange           - the range to tighten with assertions
 //
-void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP assertions, Range* pRange)
+void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
+                                     ValueNum         normalLclVN,
+                                     ValueNum         preferredBoundVN,
+                                     ASSERT_VALARG_TP assertions,
+                                     Range*           pRange,
+                                     bool             log)
 {
-    if (BitVecOps::IsEmpty(m_pCompiler->apTraits, assertions))
+    if (BitVecOps::IsEmpty(comp->apTraits, assertions))
     {
         return;
     }
@@ -663,14 +671,14 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
         return;
     }
 
-    // Walk through the "assertions" to check if the apply.
-    BitVecOps::Iter iter(m_pCompiler->apTraits, assertions);
+    // Walk through the "assertions" to check if they apply.
+    BitVecOps::Iter iter(comp->apTraits, assertions);
     unsigned        index = 0;
     while (iter.NextElem(&index))
     {
         AssertionIndex assertionIndex = GetAssertionIndex(index);
 
-        Compiler::AssertionDsc* curAssertion = m_pCompiler->optGetAssertion(assertionIndex);
+        Compiler::AssertionDsc* curAssertion = comp->optGetAssertion(assertionIndex);
 
         Limit      limit(Limit::keUndef);
         genTreeOps cmpOper             = GT_NONE;
@@ -683,7 +691,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
             ValueNumStore::CompareCheckedBoundArithInfo info;
 
             // Get i, len, cns and < as "info."
-            m_pCompiler->vnStore->GetCompareCheckedBoundArithInfo(curAssertion->op1.vn, &info);
+            comp->vnStore->GetCompareCheckedBoundArithInfo(curAssertion->op1.vn, &info);
 
             // If we don't have the same variable we are comparing against, bail.
             if (normalLclVN != info.cmpOp)
@@ -697,12 +705,12 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
             }
 
             // If the operand that operates on the bound is not constant, then done.
-            if (!m_pCompiler->vnStore->IsVNInt32Constant(info.arrOp))
+            if (!comp->vnStore->IsVNInt32Constant(info.arrOp))
             {
                 continue;
             }
 
-            int cons = m_pCompiler->vnStore->ConstantValue<int>(info.arrOp);
+            int cons = comp->vnStore->ConstantValue<int>(info.arrOp);
             limit    = Limit(Limit::keBinOpArray, info.vnBound, info.arrOper == GT_SUB ? -cons : cons);
             cmpOper  = (genTreeOps)info.cmpOper;
         }
@@ -712,7 +720,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
             ValueNumStore::CompareCheckedBoundArithInfo info;
 
             // Get the info as "i", "<" and "len"
-            m_pCompiler->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info);
+            comp->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info);
 
             // If we don't have the same variable we are comparing against, bail.
             if (normalLclVN == info.cmpOp)
@@ -736,7 +744,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
             ValueNumStore::ConstantBoundInfo info;
 
             // Get the info as "i", "<" and "100"
-            m_pCompiler->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info);
+            comp->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info);
 
             // If we don't have the same variable we are comparing against, bail.
             if (normalLclVN != info.cmpOpVN)
@@ -756,10 +764,10 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
                 continue;
             }
 
-            int cnstLimit = m_pCompiler->vnStore->CoercedConstantValue<int>(curAssertion->op2.vn);
+            int cnstLimit = comp->vnStore->CoercedConstantValue<int>(curAssertion->op2.vn);
 
             if ((cnstLimit == 0) && (curAssertion->assertionKind == Compiler::OAK_NOT_EQUAL) &&
-                m_pCompiler->vnStore->IsVNCheckedBound(curAssertion->op1.vn))
+                comp->vnStore->IsVNCheckedBound(curAssertion->op1.vn))
             {
                 // we have arr.Len != 0, so the length must be atleast one
                 limit   = Limit(Limit::keConstant, 1);
@@ -804,31 +812,31 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
 
         // Make sure the assertion is of the form != 0 or == 0 if it isn't a constant assertion.
         if (!isConstantAssertion && (curAssertion->assertionKind != Compiler::OAK_NO_THROW) &&
-            (curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT)))
+            (curAssertion->op2.vn != comp->vnStore->VNZeroForType(TYP_INT)))
         {
             continue;
         }
 #ifdef DEBUG
-        if (m_pCompiler->verbose)
+        if (comp->verbose)
         {
-            m_pCompiler->optPrintAssertion(curAssertion, assertionIndex);
+            comp->optPrintAssertion(curAssertion, assertionIndex);
         }
 #endif
 
         // Limits are sometimes made with the form vn + constant, where vn is a known constant
         // see if we can simplify this to just a constant
-        if (limit.IsBinOpArray() && m_pCompiler->vnStore->IsVNInt32Constant(limit.vn))
+        if (limit.IsBinOpArray() && comp->vnStore->IsVNInt32Constant(limit.vn))
         {
-            Limit tempLimit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue<int>(limit.vn));
+            Limit tempLimit = Limit(Limit::keConstant, comp->vnStore->ConstantValue<int>(limit.vn));
             if (tempLimit.AddConstant(limit.cns))
             {
                 limit = tempLimit;
             }
         }
 
-        ValueNum arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->GetArrayLength()->gtVNPair);
+        ValueNum arrLenVN = preferredBoundVN;
 
-        if (m_pCompiler->vnStore->IsVNConstant(arrLenVN))
+        if (comp->vnStore->IsVNConstant(arrLenVN))
         {
             // Set arrLenVN to NoVN; this will make it match the "vn" recorded on
             // constant limits (where we explicitly track the constant and don't
@@ -917,7 +925,10 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
 
             if (limit.vn != arrLenVN)
             {
-                JITDUMP("Array length VN did not match arrLen=" FMT_VN ", limit=" FMT_VN "\n", arrLenVN, limit.vn);
+                if (log)
+                {
+                    JITDUMP("Array length VN did not match arrLen=" FMT_VN ", limit=" FMT_VN "\n", arrLenVN, limit.vn);
+                }
                 continue;
             }
 
@@ -927,7 +938,10 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
             // Incoming limit doesn't tighten the existing upper limit.
             if (limCns >= curCns)
             {
-                JITDUMP("Bound limit %d doesn't tighten current bound %d\n", limCns, curCns);
+                if (log)
+                {
+                    JITDUMP("Bound limit %d doesn't tighten current bound %d\n", limCns, curCns);
+                }
                 continue;
             }
         }
@@ -954,8 +968,12 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
 
             case GT_GT:
             case GT_GE:
-                pRange->lLimit = limit;
-                // it doesn't matter if it's isUnsigned or not here - it's not negative anyway.
+                // GT/GE being unsigned creates a non-contiguous range which we can't represent
+                // using single Range object.
+                if (!isUnsigned)
+                {
+                    pRange->lLimit = limit;
+                }
                 break;
 
             case GT_EQ:
@@ -967,9 +985,13 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
                 // All other 'cmpOper' kinds leave lLimit/uLimit unchanged
                 break;
         }
-        JITDUMP("The range after edge merging:");
-        JITDUMP(pRange->ToString(m_pCompiler));
-        JITDUMP("\n");
+
+        if (log)
+        {
+            JITDUMP("The range after edge merging:");
+            JITDUMP(pRange->ToString(comp));
+            JITDUMP("\n");
+        }
     }
 }
 
diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h
@@ -686,7 +686,12 @@ class RangeCheck
     void MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange);
 
     // Inspect the assertions about the current ValueNum to refine pRange
-    void MergeEdgeAssertions(ValueNum num, ASSERT_VALARG_TP assertions, Range* pRange);
+    static void MergeEdgeAssertions(Compiler*        comp,
+                                    ValueNum         num,
+                                    ValueNum         preferredBoundVN,
+                                    ASSERT_VALARG_TP assertions,
+                                    Range*           pRange,
+                                    bool             log = true);
 
     // The maximum possible value of the given "limit". If such a value could not be determined
     // return "false". For example: CORINFO_Array_MaxLength for array length.
