Analyze reaching PHI in GetRangeFromAssertions (more branch foldings)

EgorBo · EgorBo · commit 0f0a9ecdf086 · 2026-01-24T00:45:06.000+01:00
diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
@@ -3970,19 +3970,14 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions,
     // Let's see if MergeEdgeAssertions can help us:
     if (tree->TypeIs(TYP_INT))
     {
-        Range rng = Range(Limit(Limit::keUnknown));
-        if (RangeCheck::TryGetRangeFromAssertions(this, treeVN, assertions, &rng))
+        Range rng = RangeCheck::GetRangeFromAssertions(this, treeVN, assertions);
+        if (rng.LowerLimit().GetConstant() >= 0)
         {
-            Limit lowerBound = rng.LowerLimit();
-            assert(lowerBound.IsConstant());
-            if (lowerBound.GetConstant() >= 0)
-            {
-                *isKnownNonNegative = true;
-            }
-            if (lowerBound.GetConstant() > 0)
-            {
-                *isKnownNonZero = true;
-            }
+            *isKnownNonNegative = true;
+        }
+        if (rng.LowerLimit().GetConstant() > 0)
+        {
+            *isKnownNonZero = true;
         }
     }
 }
@@ -4250,68 +4245,6 @@ GenTree* Compiler::optAssertionProp_RelOp(ASSERT_VALARG_TP assertions,
     return optAssertionPropLocal_RelOp(assertions, tree, stmt);
 }
 
-//--------------------------------------------------------------------------------
-// optVisitReachingAssertions: given a vn, call the specified callback function on all
-//    the assertions that reach it via PHI definitions if any.
-//
-// Arguments:
-//    vn         - The vn to visit all the reaching assertions for
-//    argVisitor - The callback function to call on the vn and its reaching assertions
-//
-// Return Value:
-//    AssertVisit::Aborted  - an argVisitor returned AssertVisit::Abort, we stop the walk and return
-//    AssertVisit::Continue - all argVisitor returned AssertVisit::Continue
-//
-template <typename TAssertVisitor>
-Compiler::AssertVisit Compiler::optVisitReachingAssertions(ValueNum vn, TAssertVisitor argVisitor)
-{
-    VNPhiDef phiDef;
-    if (!vnStore->GetPhiDef(vn, &phiDef))
-    {
-        // We assume that the caller already checked assertions for the current block, so we're
-        // interested only in assertions for PHI definitions.
-        return AssertVisit::Abort;
-    }
-
-    LclSsaVarDsc*        ssaDef = lvaGetDesc(phiDef.LclNum)->GetPerSsaData(phiDef.SsaDef);
-    GenTreeLclVarCommon* node   = ssaDef->GetDefNode();
-    assert(node->IsPhiDefn());
-
-    // Keep track of the set of phi-preds
-    //
-    BitVecTraits traits(fgBBNumMax + 1, this);
-    BitVec       visitedBlocks = BitVecOps::MakeEmpty(&traits);
-
-    for (GenTreePhi::Use& use : node->Data()->AsPhi()->Uses())
-    {
-        GenTreePhiArg* phiArg     = use.GetNode()->AsPhiArg();
-        const ValueNum phiArgVN   = vnStore->VNConservativeNormalValue(phiArg->gtVNPair);
-        ASSERT_TP      assertions = optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB);
-        if (argVisitor(phiArgVN, assertions) == AssertVisit::Abort)
-        {
-            // The visitor wants to abort the walk.
-            return AssertVisit::Abort;
-        }
-        BitVecOps::AddElemD(&traits, visitedBlocks, phiArg->gtPredBB->bbNum);
-    }
-
-    // Verify the set of phi-preds covers the set of block preds
-    //
-    for (BasicBlock* const pred : ssaDef->GetBlock()->PredBlocks())
-    {
-        if (!BitVecOps::IsMember(&traits, visitedBlocks, pred->bbNum))
-        {
-            JITDUMP("... optVisitReachingAssertions in " FMT_BB ": pred " FMT_BB " not a phi-pred\n",
-                    ssaDef->GetBlock()->bbNum, pred->bbNum);
-
-            // We missed examining a block pred. Fail the phi inference.
-            //
-            return AssertVisit::Abort;
-        }
-    }
-    return AssertVisit::Continue;
-}
-
 //------------------------------------------------------------------------
 // optAssertionProp: try and optimize a relop via assertion propagation
 //
@@ -4400,23 +4333,17 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions,
     ValueNum op1VN = vnStore->VNConservativeNormalValue(op1->gtVNPair);
     ValueNum op2VN = vnStore->VNConservativeNormalValue(op2->gtVNPair);
 
-    // See if we can fold "X relop CNS" using TryGetRangeFromAssertions.
-    int op2cns;
-    if (op1->TypeIs(TYP_INT) && op2->TypeIs(TYP_INT) && vnStore->IsVNIntegralConstant(op2VN, &op2cns))
+    if (op1->TypeIs(TYP_INT) && op2->TypeIs(TYP_INT))
     {
-        // NOTE: we can call TryGetRangeFromAssertions for op2 as well if we want, but it's not cheap.
-        Range rng1 = Range(Limit(Limit::keUnknown));
-        Range rng2 = Range(Limit(Limit::keConstant, op2cns));
+        Range rng1 = RangeCheck::GetRangeFromAssertions(this, op1VN, assertions);
+        Range rng2 = RangeCheck::GetRangeFromAssertions(this, op2VN, assertions);
 
-        if (RangeCheck::TryGetRangeFromAssertions(this, op1VN, assertions, &rng1))
+        RangeOps::RelationKind kind = RangeOps::EvalRelop(tree->OperGet(), tree->IsUnsigned(), rng1, rng2);
+        if ((kind != RangeOps::RelationKind::Unknown))
         {
-            RangeOps::RelationKind kind = RangeOps::EvalRelop(tree->OperGet(), tree->IsUnsigned(), rng1, rng2);
-            if ((kind != RangeOps::RelationKind::Unknown))
-            {
-                newTree = kind == RangeOps::RelationKind::AlwaysTrue ? gtNewTrue() : gtNewFalse();
-                newTree = gtWrapWithSideEffects(newTree, tree, GTF_ALL_EFFECT);
-                return optAssertionProp_Update(newTree, tree, stmt);
-            }
+            newTree = kind == RangeOps::RelationKind::AlwaysTrue ? gtNewTrue() : gtNewFalse();
+            newTree = gtWrapWithSideEffects(newTree, tree, GTF_ALL_EFFECT);
+            return optAssertionProp_Update(newTree, tree, stmt);
         }
     }
 
@@ -5346,10 +5273,10 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree
             std::swap(funcApp.m_args[0], funcApp.m_args[1]);
         }
 
-        Range rng = Range(Limit(Limit::keUnknown));
-        if ((funcApp.m_args[0] == vnCurLen) && vnStore->IsVNInt32Constant(funcApp.m_args[1]) &&
-            RangeCheck::TryGetRangeFromAssertions(this, vnCurLen, assertions, &rng) && rng.LowerLimit().IsConstant())
+        Range(Limit(Limit::keUnknown));
+        if ((funcApp.m_args[0] == vnCurLen) && vnStore->IsVNInt32Constant(funcApp.m_args[1]))
         {
+            Range rng = RangeCheck::GetRangeFromAssertions(this, vnCurLen, assertions);
             // Lower known limit of ArrLen:
             const int lenLowerLimit = rng.LowerLimit().GetConstant();
 
diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -8129,8 +8129,68 @@ class Compiler
         Continue,
         Abort,
     };
+
+    //--------------------------------------------------------------------------------
+    // optVisitReachingAssertions: given a vn, call the specified callback function on all
+    //    the assertions that reach it via PHI definitions if any.
+    //
+    // Arguments:
+    //    vn         - The vn to visit all the reaching assertions for
+    //    argVisitor - The callback function to call on the vn and its reaching assertions
+    //
+    // Return Value:
+    //    AssertVisit::Aborted  - an argVisitor returned AssertVisit::Abort, we stop the walk and return
+    //    AssertVisit::Continue - all argVisitor returned AssertVisit::Continue
+    //
     template <typename TAssertVisitor>
-    AssertVisit optVisitReachingAssertions(ValueNum vn, TAssertVisitor argVisitor);
+    AssertVisit optVisitReachingAssertions(ValueNum vn, TAssertVisitor argVisitor)
+    {
+        VNPhiDef phiDef;
+        if (!vnStore->GetPhiDef(vn, &phiDef))
+        {
+            // We assume that the caller already checked assertions for the current block, so we're
+            // interested only in assertions for PHI definitions.
+            return AssertVisit::Abort;
+        }
+
+        LclSsaVarDsc*        ssaDef = lvaGetDesc(phiDef.LclNum)->GetPerSsaData(phiDef.SsaDef);
+        GenTreeLclVarCommon* node   = ssaDef->GetDefNode();
+        assert(node->IsPhiDefn());
+
+        // Keep track of the set of phi-preds
+        //
+        BitVecTraits traits(fgBBNumMax + 1, this);
+        BitVec       visitedBlocks = BitVecOps::MakeEmpty(&traits);
+
+        for (GenTreePhi::Use& use : node->Data()->AsPhi()->Uses())
+        {
+            GenTreePhiArg* phiArg     = use.GetNode()->AsPhiArg();
+            const ValueNum phiArgVN   = vnStore->VNConservativeNormalValue(phiArg->gtVNPair);
+            ASSERT_TP      assertions = optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB);
+            if (argVisitor(phiArgVN, assertions) == AssertVisit::Abort)
+            {
+                // The visitor wants to abort the walk.
+                return AssertVisit::Abort;
+            }
+            BitVecOps::AddElemD(&traits, visitedBlocks, phiArg->gtPredBB->bbNum);
+        }
+
+        // Verify the set of phi-preds covers the set of block preds
+        //
+        for (BasicBlock* const pred : ssaDef->GetBlock()->PredBlocks())
+        {
+            if (!BitVecOps::IsMember(&traits, visitedBlocks, pred->bbNum))
+            {
+                JITDUMP("... optVisitReachingAssertions in " FMT_BB ": pred " FMT_BB " not a phi-pred\n",
+                        ssaDef->GetBlock()->bbNum, pred->bbNum);
+
+                // We missed examining a block pred. Fail the phi inference.
+                //
+                return AssertVisit::Abort;
+            }
+        }
+        return AssertVisit::Continue;
+    }
 
     void optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions,
                                           GenTree*         tree,
diff --git a/src/coreclr/jit/rangecheck.cpp b/src/coreclr/jit/rangecheck.cpp
@@ -637,29 +637,30 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
 }
 
 //------------------------------------------------------------------------
-// TryGetRangeFromAssertions: Cheaper version of TryGetRange that is based purely on assertions
+// GetRangeFromAssertions: Cheaper version of TryGetRange that is based purely on assertions
 //    and does not require a full range analysis based on SSA.
 //
 // Arguments:
 //    comp             - the compiler instance
 //    num              - the value number to analyze range for
 //    assertions       - the assertions to use
-//    pRange           - the range to tighten with assertions
 //
 // Return Value:
-//    True if the range was successfully computed
+//    The computed range
 //
-bool RangeCheck::TryGetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, Range* pRange)
+Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, int budget)
 {
-    assert(pRange != nullptr);
-    assert(pRange->LowerLimit().IsUnknown());
-    assert(pRange->UpperLimit().IsUnknown());
+    // Start with the widest possible constant range.
+    Range result = Range(Limit(Limit::keConstant, INT32_MIN), Limit(Limit::keConstant, INT32_MAX));
 
-    if (num == ValueNumStore::NoVN)
+    if ((num == ValueNumStore::NoVN) || (budget <= 0))
     {
-        return false;
+        return result;
     }
 
+    // Currently, we only handle int32 and smaller integer types.
+    assert(varTypeIsInt(comp->vnStore->TypeOfVN(num)) || varTypeIsSmall(comp->vnStore->TypeOfVN(num)));
+
     //
     // First, let's see if we can tighten the range based on VN information.
     //
@@ -668,14 +669,9 @@ bool RangeCheck::TryGetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_
     int cns;
     if (comp->vnStore->IsVNIntegralConstant(num, &cns))
     {
-        pRange->lLimit = Limit(Limit::keConstant, cns);
-        pRange->uLimit = Limit(Limit::keConstant, cns);
-        return true;
+        return Range(Limit(Limit::keConstant, cns));
     }
 
-    // Start with the widest possible constant range.
-    Range result = Range(Limit(Limit::keConstant, INT32_MIN), Limit(Limit::keConstant, INT32_MAX));
-
     VNFuncApp funcApp;
     if (comp->vnStore->GetVNFunc(num, &funcApp))
     {
@@ -718,6 +714,16 @@ bool RangeCheck::TryGetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_
                 }
                 break;
 
+            case VNF_NEG:
+            {
+                Range r1            = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget);
+                Range unaryOpResult = RangeOps::Negate(r1);
+
+                // We can use the result only if it never overflows.
+                result = unaryOpResult.IsConstantRange() ? unaryOpResult : result;
+                break;
+            }
+
             case VNF_LSH:
             case VNF_ADD:
             case VNF_MUL:
@@ -728,49 +734,41 @@ bool RangeCheck::TryGetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_
             case VNF_UMOD:
             {
                 // Get ranges of both operands and perform the same operation on the ranges.
-                Range r1 = Range(Limit(Limit::keUnknown));
-                Range r2 = Range(Limit(Limit::keUnknown));
-                if (TryGetRangeFromAssertions(comp, funcApp.m_args[0], assertions, &r1) &&
-                    TryGetRangeFromAssertions(comp, funcApp.m_args[1], assertions, &r2))
+                Range r1          = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget);
+                Range r2          = GetRangeFromAssertions(comp, funcApp.m_args[1], assertions, --budget);
+                Range binOpResult = Range(Limit(Limit::keUnknown));
+                switch (funcApp.m_func)
                 {
-                    Range binOpResult = Range(Limit(Limit::keUnknown));
-                    switch (funcApp.m_func)
-                    {
-                        case VNF_ADD:
-                            binOpResult = RangeOps::Add(r1, r2);
-                            break;
-                        case VNF_MUL:
-                            binOpResult = RangeOps::Multiply(r1, r2);
-                            break;
-                        case VNF_AND:
-                            binOpResult = RangeOps::And(r1, r2);
-                            break;
-                        case VNF_OR:
-                            binOpResult = RangeOps::Or(r1, r2);
-                            break;
-                        case VNF_LSH:
-                            binOpResult = RangeOps::ShiftLeft(r1, r2);
-                            break;
-                        case VNF_RSH:
-                            binOpResult = RangeOps::ShiftRight(r1, r2, /*logical*/ false);
-                            break;
-                        case VNF_RSZ:
-                            binOpResult = RangeOps::ShiftRight(r1, r2, /*logical*/ true);
-                            break;
-                        case VNF_UMOD:
-                            binOpResult = RangeOps::UnsignedMod(r1, r2);
-                            break;
-                        default:
-                            unreached();
-                    }
-
-                    if (binOpResult.IsConstantRange())
-                    {
-                        result = binOpResult;
-                    }
-                    // if the result is unknown (or may overflow), we'll just analyze the binop itself based on the
-                    // assertions
+                    case VNF_ADD:
+                        binOpResult = RangeOps::Add(r1, r2);
+                        break;
+                    case VNF_MUL:
+                        binOpResult = RangeOps::Multiply(r1, r2);
+                        break;
+                    case VNF_AND:
+                        binOpResult = RangeOps::And(r1, r2);
+                        break;
+                    case VNF_OR:
+                        binOpResult = RangeOps::Or(r1, r2);
+                        break;
+                    case VNF_LSH:
+                        binOpResult = RangeOps::ShiftLeft(r1, r2);
+                        break;
+                    case VNF_RSH:
+                        binOpResult = RangeOps::ShiftRight(r1, r2, /*logical*/ false);
+                        break;
+                    case VNF_RSZ:
+                        binOpResult = RangeOps::ShiftRight(r1, r2, /*logical*/ true);
+                        break;
+                    case VNF_UMOD:
+                        binOpResult = RangeOps::UnsignedMod(r1, r2);
+                        break;
+                    default:
+                        unreached();
                 }
+
+                // We can use the result only if it never overflows.
+                result = binOpResult.IsConstantRange() ? binOpResult : result;
                 break;
             }
 
@@ -807,10 +805,28 @@ bool RangeCheck::TryGetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_
         }
     }
 
+    Range phiRange = Range(Limit(Limit::keUndef));
+    if (comp->optVisitReachingAssertions(num,
+                                         [comp, &phiRange, &budget](ValueNum reachingVN, ASSERT_TP reachingAssertions) {
+        // call GetRangeFromAssertions for each reaching VN using reachingAssertions
+        Range edgeRange = GetRangeFromAssertions(comp, reachingVN, reachingAssertions, --budget);
+
+        // If phiRange is not yet set, set it to the first edgeRange
+        // else merge it with the new edgeRange. Example: [10..100] U [50..150] = [10..150]
+        phiRange = phiRange.LowerLimit().IsUndef() ? edgeRange : RangeOps::Merge(phiRange, edgeRange, false);
+
+        // if any edge produces a non-constant range, we abort further processing
+        // We also give up if the range is full, as it won't help tighten the result.
+        return edgeRange.IsConstantRange() && !edgeRange.IsFullRange() ? Compiler::AssertVisit::Continue
+                                                                       : Compiler::AssertVisit::Abort;
+    }) == Compiler::AssertVisit::Continue)
+    {
+        result = phiRange;
+    }
+
     MergeEdgeAssertions(comp, num, ValueNumStore::NoVN, assertions, &result, false);
     assert(result.IsConstantRange());
-    *pRange = result;
-    return true;
+    return result;
 }
 
 //------------------------------------------------------------------------
diff --git a/src/coreclr/jit/rangecheck.h b/src/coreclr/jit/rangecheck.h
