JIT: Switch StaysWithinManagedObject to peel offsets from VNs (#105169

)

The SCEV analysis does not care about the value of something once it is
seen to be invariant inside the loop we are currently analyzing. This
was problematic for this logic that tries to peel additions away from
offsets; for arm64, we may have hoisted `array + 0x10` outside the loop,
which would cause us to fail to get back to the base array.

Switch the reasoning to use VNs and peel the offsets from the VNs
instead.

No x64 diffs are expected as we do not hoist the `array + 0x10` out of
the loop there. Improvements expected on arm64 where we can now prove
that a "full" strength reduction is allowable more often.

src/coreclr/jit/inductionvariableopts.cpp

@@ -1845,16 +1845,32 @@ bool StrengthReductionContext::CheckAdvancedCursors(ArrayStack<CursorInfo>* curs
 //
 bool StrengthReductionContext::StaysWithinManagedObject(ArrayStack<CursorInfo>* cursors, ScevAddRec* addRec)
 {
-    int64_t offset;
-    Scev*   baseScev = addRec->Start->PeelAdditions(&offset);
-    offset           = static_cast<target_ssize_t>(offset);
+    ValueNumPair addRecStartVNP = m_scevContext.MaterializeVN(addRec->Start);
+    if (!addRecStartVNP.BothDefined())
+    {
+        return false;
+    }
+
+    ValueNumPair   addRecStartBase    = addRecStartVNP;
+    target_ssize_t offsetLiberal      = 0;
+    target_ssize_t offsetConservative = 0;
+    m_comp->vnStore->PeelOffsets(addRecStartBase.GetLiberalAddr(), &offsetLiberal);
+    m_comp->vnStore->PeelOffsets(addRecStartBase.GetConservativeAddr(), &offsetConservative);
+
+    if (offsetLiberal != offsetConservative)
+    {
+        return false;
+    }
+
+    target_ssize_t offset = offsetLiberal;
 
-    // We only support arrays and strings here. To strength reduce Span<T>
-    // accesses we need additional properies on the range designated by a
-    // Span<T> that we currently do not specify, or we need to prove that the
-    // byref we may form in the IV update would have been formed anyway by the
-    // loop.
-    if (!baseScev->OperIs(ScevOper::Local) || !baseScev->TypeIs(TYP_REF))
+    // We only support objects here (targeting array/strings). To strength
+    // reduce Span<T> accesses we need additional properties on the range
+    // designated by a Span<T> that we currently do not specify, or we need to
+    // prove that the byref we may form in the IV update would have been formed
+    // anyway by the loop.
+    if ((m_comp->vnStore->TypeOfVN(addRecStartBase.GetConservative()) != TYP_REF) ||
+        (m_comp->vnStore->TypeOfVN(addRecStartBase.GetLiberal()) != TYP_REF))
     {
         return false;
     }
@@ -1888,22 +1904,14 @@ bool StrengthReductionContext::StaysWithinManagedObject(ArrayStack<CursorInfo>*
         return false;
     }
 
-    ScevLocal* local = (ScevLocal*)baseScev;
-
-    ValueNumPair vnp = m_scevContext.MaterializeVN(baseScev);
-    if (!vnp.BothDefined())
-    {
-        return false;
-    }
-
     BasicBlock* preheader = m_loop->EntryEdge(0)->getSourceBlock();
-    if (!m_comp->optAssertionVNIsNonNull(vnp.GetConservative(), preheader->bbAssertionOut))
+    if (!m_comp->optAssertionVNIsNonNull(addRecStartBase.GetConservative(), preheader->bbAssertionOut))
     {
         return false;
     }
 
-    // We have a non-null array/string. Check that the 'start' offset looks
-    // fine. TODO: We could also use assertions on the length of the
+    // We have a non-null object as the base. Check that the 'start' offset
+    // looks fine. TODO: We could also use assertions on the length of the
     // array/string. E.g. if we know the length of the array is > 3, then we
     // can allow the add rec to have a later start. Maybe range check can be
     // used?
@@ -1914,7 +1922,7 @@ bool StrengthReductionContext::StaysWithinManagedObject(ArrayStack<CursorInfo>*
 
     // Now see if we have a bound that guarantees that we iterate fewer times
     // than the array/string's length.
-    ValueNum arrLengthVN = m_comp->vnStore->VNForFunc(TYP_INT, VNF_ARR_LENGTH, vnp.GetLiberal());
+    ValueNum arrLengthVN = m_comp->vnStore->VNForFunc(TYP_INT, VNF_ARR_LENGTH, addRecStartBase.GetLiberal());
 
     for (int i = 0; i < m_backEdgeBounds.Height(); i++)
     {

src/coreclr/jit/valuenum.cpp

@@ -14998,3 +14998,39 @@ CORINFO_CLASS_HANDLE ValueNumStore::GetObjectType(ValueNum vn, bool* pIsExact, b
 
     return NO_CLASS_HANDLE;
 }
+
+//--------------------------------------------------------------------------------
+// PeelOffsets: Peel all additions with a constant offset away from the
+// specified VN.
+//
+// Arguments:
+//    vn     - [in, out] The VN. Will be modified to the base VN that the offsets are added to.
+//    offset - [out] The offsets peeled out of the VNF_ADD funcs.
+//
+void ValueNumStore::PeelOffsets(ValueNum* vn, target_ssize_t* offset)
+{
+#ifdef DEBUG
+    var_types vnType = TypeOfVN(*vn);
+    assert((vnType == TYP_I_IMPL) || (vnType == TYP_REF) || (vnType == TYP_BYREF));
+#endif
+
+    *offset = 0;
+    VNFuncApp app;
+    while (GetVNFunc(*vn, &app) && (app.m_func == VNF_ADD))
+    {
+        if (IsVNConstantNonHandle(app.m_args[0]))
+        {
+            *offset += ConstantValue<target_ssize_t>(app.m_args[0]);
+            *vn = app.m_args[1];
+        }
+        else if (IsVNConstantNonHandle(app.m_args[1]))
+        {
+            *offset += ConstantValue<target_ssize_t>(app.m_args[1]);
+            *vn = app.m_args[0];
+        }
+        else
+        {
+            break;
+        }
+    }
+}

src/coreclr/jit/valuenum.h

@@ -512,6 +512,8 @@ class ValueNumStore
 
     CORINFO_CLASS_HANDLE GetObjectType(ValueNum vn, bool* pIsExact, bool* pIsNonNull);
 
+    void PeelOffsets(ValueNum* vn, target_ssize_t* offset);
+
     // And the single constant for an object reference type.
     static ValueNum VNForNull()
     {