[llvm] [LAA] Fix off-by-EltSize in negative-step deref bounds check (PR #211964)

[Aleksandr Popov via llvm-commits]

================
@@ -301,27 +311,12 @@ static bool evaluatePtrAddRecAtMaxBTCWillNotWrap(
   if (!AccessedBytes)
     return false;
 
-  // Compute MaxOffset per direction: exclusive upper offset of the
-  // accessed range.
-  const SCEV *MaxOffset;
-  if (IsKnownNonNegative) {
-    MaxOffset = addSCEVNoOverflow(StartOffset, AccessedBytes, SE);
-    if (!MaxOffset)
-      return false;
-    DerefBytesSCEV = SE.applyLoopGuards(DerefBytesSCEV, *LoopGuards);
-  } else {
-    // FIXME: two independent off-by-EltSize bugs on this branch:
-    //  1. StartOffset here is actually the HIGHEST offset, because it is
-    //     computed from AR->getStart() rather than
-    //     AR->evaluateAtIteration(MaxBTC, SE) (see FIXME above).
-    //  2. The lower check is over-strict by EltSize and the upper is
-    //     under-counted by EltSize.
-    assert(SE.isKnownNegative(Step) && "must be known negative");
-    if (!SE.isKnownPredicate(CmpInst::ICMP_SGE, StartOffset, AccessedBytes))
-      return false;
-    MaxOffset = StartOffset;
-  }
-  // MaxOffset must not exceed the deref-region end.
+  // Exclusive upper offset of the accessed range.
+  const SCEV *MaxOffset = addSCEVNoOverflow(LowestOffset, AccessedBytes, SE);
+  if (!MaxOffset)
+    return false;
+  DerefBytesSCEV = SE.applyLoopGuards(DerefBytesSCEV, *LoopGuards);
----------------
aleks-tmb wrote:

Agree, will add it as a follow-up.

Also, I added one more test for vectorization that's independent of that change, since the current one depends on it. I extracted the tests into a precommit patch, if that makes sense: https://github.com/llvm/llvm-project/pull/228062

https://github.com/llvm/llvm-project/pull/211964