[llvm] [DA] do not handle array accesses of different offsets (PR #123436)

[Michael Kruse via llvm-commits]

================
@@ -3569,6 +3569,123 @@ bool DependenceInfo::invalidate(Function &F, const PreservedAnalyses &PA,
          Inv.invalidate<LoopAnalysis>(F, PA);
 }
 
+// Check that memory access offsets in V are multiples of array element size
+// EltSize. Param records the first parametric expression. If the scalar
+// evolution V contains two or more parameters, we check that the subsequent
+// parametric expressions are multiples of the first parametric expression
+// Param.
+static bool checkOffsets(ScalarEvolution *SE, const SCEV *V, const SCEV *&Param,
+                         uint64_t EltSize) {
+  if (auto *AddRec = dyn_cast<SCEVAddRecExpr>(V)) {
+    if (!checkOffsets(SE, AddRec->getStart(), Param, EltSize))
+      return false;
+    return checkOffsets(SE, AddRec->getStepRecurrence(*SE), Param, EltSize);
+  }
+  if (auto *Cst = dyn_cast<SCEVConstant>(V)) {
+    APInt C = Cst->getAPInt();
+
+    // For example, alias_with_different_offsets in
+    // test/Analysis/DependenceAnalysis/DifferentOffsets.ll accesses "%A + 2":
+    //   %arrayidx = getelementptr inbounds i8, ptr %A, i64 2
+    //   store i32 42, ptr %arrayidx, align 1
+    // which is writing an i32, i.e., EltSize = 4 bytes, with an offset C = 2.
+    // checkOffsets returns false, as the offset C=2 is not a multiple of 4.
+    return C.srem(EltSize) == 0;
+  }
+
+  // Use a lambda helper function to check V for parametric expressions.
+  // Param records the first parametric expression. If the scalar evolution V
+  // contains two or more parameters, we check that the subsequent parametric
+  // expressions are multiples of the first parametric expression Param.
+  auto checkParamsMultipleOfSize = [&](const SCEV *V,
+                                       const SCEV *&Param) -> bool {
+    if (EltSize == 1)
+      return true;
+    if (!Param) {
+      Param = V;
+      return true;
----------------
Meinersbur wrote:

> If a parameter (ssa name invariant in loop) name %n is used as offset, then we assume that %n is a multiple of the array element size. There isn't even a check that `%n` is loop-invariant.

That ssumption that obviously does not necessarily hold true.

I think what you mean is that DA is assuming that `%n` is part of all index expressions and therefore has the same offset everywhere. If some index expressions use `%m` instead, they may have different relative offsets. 

I do not think this is sound. Some index expressions may also use `%n + %n`, `%n + 1`, or no `%n` at all in which case they may have different relative offsets already. Alternatively, I may not have understood the logic here.

If `%n` (or any other subexpression) is invariant to all loops and occurs in every index, I think it could considered part of the base pointer. `ScalarEvolution::getPointerBase` atm looks into all SCEVAddExpr disregarding whether operands are loop invariant. Maybe that's a viable approach.



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