Revert "Use rangecheck in assertprop (#112766)" (#112872)

AndyAyersMS · web-flow · commit db681fb307d7 · 2025-02-24T15:48:23.000-08:00
This reverts commit 6022adf.
diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
@@ -11,7 +11,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
 */
 
 #include "jitpch.h"
-#include "rangecheck.h"
 #ifdef _MSC_VER
 #pragma hdrstop
 #endif
@@ -4126,69 +4125,6 @@ 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;
-                }
-            }
-        }
-    }
 }
 
 //------------------------------------------------------------------------
@@ -4915,6 +4851,12 @@ 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))
@@ -4928,12 +4870,6 @@ 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(m_pCompiler, arrLenVn, arrLenVn, block->bbAssertionIn, &arrLength);
+            MergeEdgeAssertions(arrLenVn, block->bbAssertionIn, &arrLength);
             if (arrLength.lLimit.IsConstant())
             {
                 arrSize = arrLength.lLimit.GetConstant();
@@ -640,28 +640,20 @@ 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);
-    ValueNum      arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->GetArrayLength()->gtVNPair);
-    MergeEdgeAssertions(m_pCompiler, normalLclVN, arrLenVN, assertions, pRange);
+    MergeEdgeAssertions(normalLclVN, assertions, pRange);
 }
 
 //------------------------------------------------------------------------
 // MergeEdgeAssertions: Merge assertions on the edge flowing into the block about a variable
 //
 // Arguments:
-//    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
+//    normalLclVN - the value number to look for assertions for
+//    assertions - the assertions to use
+//    pRange - the range to tighten with assertions
 //
-void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
-                                     ValueNum         normalLclVN,
-                                     ValueNum         preferredBoundVN,
-                                     ASSERT_VALARG_TP assertions,
-                                     Range*           pRange,
-                                     bool             log)
+void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP assertions, Range* pRange)
 {
-    if (BitVecOps::IsEmpty(comp->apTraits, assertions))
+    if (BitVecOps::IsEmpty(m_pCompiler->apTraits, assertions))
     {
         return;
     }
@@ -671,14 +663,14 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
         return;
     }
 
-    // Walk through the "assertions" to check if they apply.
-    BitVecOps::Iter iter(comp->apTraits, assertions);
+    // Walk through the "assertions" to check if the apply.
+    BitVecOps::Iter iter(m_pCompiler->apTraits, assertions);
     unsigned        index = 0;
     while (iter.NextElem(&index))
     {
         AssertionIndex assertionIndex = GetAssertionIndex(index);
 
-        Compiler::AssertionDsc* curAssertion = comp->optGetAssertion(assertionIndex);
+        Compiler::AssertionDsc* curAssertion = m_pCompiler->optGetAssertion(assertionIndex);
 
         Limit      limit(Limit::keUndef);
         genTreeOps cmpOper             = GT_NONE;
@@ -691,7 +683,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
             ValueNumStore::CompareCheckedBoundArithInfo info;
 
             // Get i, len, cns and < as "info."
-            comp->vnStore->GetCompareCheckedBoundArithInfo(curAssertion->op1.vn, &info);
+            m_pCompiler->vnStore->GetCompareCheckedBoundArithInfo(curAssertion->op1.vn, &info);
 
             // If we don't have the same variable we are comparing against, bail.
             if (normalLclVN != info.cmpOp)
@@ -705,12 +697,12 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
             }
 
             // If the operand that operates on the bound is not constant, then done.
-            if (!comp->vnStore->IsVNInt32Constant(info.arrOp))
+            if (!m_pCompiler->vnStore->IsVNInt32Constant(info.arrOp))
             {
                 continue;
             }
 
-            int cons = comp->vnStore->ConstantValue<int>(info.arrOp);
+            int cons = m_pCompiler->vnStore->ConstantValue<int>(info.arrOp);
             limit    = Limit(Limit::keBinOpArray, info.vnBound, info.arrOper == GT_SUB ? -cons : cons);
             cmpOper  = (genTreeOps)info.cmpOper;
         }
@@ -720,7 +712,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
             ValueNumStore::CompareCheckedBoundArithInfo info;
 
             // Get the info as "i", "<" and "len"
-            comp->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info);
+            m_pCompiler->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info);
 
             // If we don't have the same variable we are comparing against, bail.
             if (normalLclVN == info.cmpOp)
@@ -744,7 +736,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
             ValueNumStore::ConstantBoundInfo info;
 
             // Get the info as "i", "<" and "100"
-            comp->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info);
+            m_pCompiler->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info);
 
             // If we don't have the same variable we are comparing against, bail.
             if (normalLclVN != info.cmpOpVN)
@@ -764,10 +756,10 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
                 continue;
             }
 
-            int cnstLimit = comp->vnStore->CoercedConstantValue<int>(curAssertion->op2.vn);
+            int cnstLimit = m_pCompiler->vnStore->CoercedConstantValue<int>(curAssertion->op2.vn);
 
             if ((cnstLimit == 0) && (curAssertion->assertionKind == Compiler::OAK_NOT_EQUAL) &&
-                comp->vnStore->IsVNCheckedBound(curAssertion->op1.vn))
+                m_pCompiler->vnStore->IsVNCheckedBound(curAssertion->op1.vn))
             {
                 // we have arr.Len != 0, so the length must be atleast one
                 limit   = Limit(Limit::keConstant, 1);
@@ -812,31 +804,31 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
 
         // 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 != comp->vnStore->VNZeroForType(TYP_INT)))
+            (curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT)))
         {
             continue;
         }
 #ifdef DEBUG
-        if (comp->verbose)
+        if (m_pCompiler->verbose)
         {
-            comp->optPrintAssertion(curAssertion, assertionIndex);
+            m_pCompiler->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() && comp->vnStore->IsVNInt32Constant(limit.vn))
+        if (limit.IsBinOpArray() && m_pCompiler->vnStore->IsVNInt32Constant(limit.vn))
         {
-            Limit tempLimit = Limit(Limit::keConstant, comp->vnStore->ConstantValue<int>(limit.vn));
+            Limit tempLimit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue<int>(limit.vn));
             if (tempLimit.AddConstant(limit.cns))
             {
                 limit = tempLimit;
             }
         }
 
-        ValueNum arrLenVN = preferredBoundVN;
+        ValueNum arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->GetArrayLength()->gtVNPair);
 
-        if (comp->vnStore->IsVNConstant(arrLenVN))
+        if (m_pCompiler->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
@@ -925,10 +917,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
 
             if (limit.vn != arrLenVN)
             {
-                if (log)
-                {
-                    JITDUMP("Array length VN did not match arrLen=" FMT_VN ", limit=" FMT_VN "\n", arrLenVN, limit.vn);
-                }
+                JITDUMP("Array length VN did not match arrLen=" FMT_VN ", limit=" FMT_VN "\n", arrLenVN, limit.vn);
                 continue;
             }
 
@@ -938,10 +927,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
             // Incoming limit doesn't tighten the existing upper limit.
             if (limCns >= curCns)
             {
-                if (log)
-                {
-                    JITDUMP("Bound limit %d doesn't tighten current bound %d\n", limCns, curCns);
-                }
+                JITDUMP("Bound limit %d doesn't tighten current bound %d\n", limCns, curCns);
                 continue;
             }
         }
@@ -968,12 +954,8 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
 
             case GT_GT:
             case GT_GE:
-                // GT/GE being unsigned creates a non-contiguous range which we can't represent
-                // using single Range object.
-                if (!isUnsigned)
-                {
-                    pRange->lLimit = limit;
-                }
+                pRange->lLimit = limit;
+                // it doesn't matter if it's isUnsigned or not here - it's not negative anyway.
                 break;
 
             case GT_EQ:
@@ -985,13 +967,9 @@ void RangeCheck::MergeEdgeAssertions(Compiler*        comp,
                 // All other 'cmpOper' kinds leave lLimit/uLimit unchanged
                 break;
         }
-
-        if (log)
-        {
-            JITDUMP("The range after edge merging:");
-            JITDUMP(pRange->ToString(comp));
-            JITDUMP("\n");
-        }
+        JITDUMP("The range after edge merging:");
+        JITDUMP(pRange->ToString(m_pCompiler));
+        JITDUMP("\n");
     }
 }
 
diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h
@@ -686,12 +686,7 @@ class RangeCheck
     void MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange);
 
     // Inspect the assertions about the current ValueNum to refine pRange
-    static void MergeEdgeAssertions(Compiler*        comp,
-                                    ValueNum         num,
-                                    ValueNum         preferredBoundVN,
-                                    ASSERT_VALARG_TP assertions,
-                                    Range*           pRange,
-                                    bool             log = true);
+    void MergeEdgeAssertions(ValueNum num, ASSERT_VALARG_TP assertions, Range* pRange);
 
     // 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.
