[PATCH] D75887: [SCEV] Add support for GEPs over scalable vectors.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:3504
   const SCEV *TotalOffset = getZero(IntIdxTy);
   // The array size is unimportant. The first thing we do on CurTy is getting
   // its element type.
----------------
nit: this comment is now stale.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:3522
       // Update CurTy to its element type.
-      CurTy = cast<SequentialType>(CurTy)->getElementType();
+      if (CurTy == GEP->getType())
+        CurTy = GEP->getSourceElementType();
----------------
Is this equivalent to:
```
  auto *VecTy = dyn_cast<VectorType>(CurTy);
  if (VecTy && VecTy->getVectorIsScalable()) {
    assert (CurTy == GEP->getType());
    CurTy = GEP->getSourceElementType();
  }else
    CurTy = cast<SequentialType>(CurTy)->getElementType();
```
?
Not sure if that's necessarily better, but otherwise having a comment that describes the reason for handling the `CurTy == GEP->getType()` case differently would be useful.


================
Comment at: llvm/test/Analysis/ScalarEvolution/scalable-vector.ll:4
+; CHECK: %1 = getelementptr <vscale x 4 x i32>, <vscale x 4 x i32>* null, i32 3
+; CHECK: -->  (3 * sizeof(<vscale x 4 x i32>)) U: [0,-15) S: [-9223372036854775808,9223372036854775793)
+; CHECK: %2 = getelementptr <vscale x 1 x i64>, <vscale x 1 x i64>* %p, i32 1
----------------
`vscale` is guaranteed to be >= 1, so the lower bound of the range should probably still be 48 rather than 0.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75887/new/

https://reviews.llvm.org/D75887