[PATCH] D109746: [AA] Teach BasicAA to recognize basic GEP range information.

[Clement Courbet via Phabricator via llvm-commits]

courbet marked 2 inline comments as done.
courbet added a comment.

Thanks a lot for the review.



================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1234
+          if (Var.ZExtBits)
+            R = R.zeroExtend(R.getBitWidth() + Var.ZExtBits);
+          VarIndexRange = R.sextOrTrunc(Var.Scale.getBitWidth())
----------------
nikic wrote:
> It's possible for both SExtBits and ZExtBits to be non-zero, so this should be extending to `R.getBitWidth() + Var.SExtBits + Var.ZExtBits`.
If that's the case, `R` has been sign-extended above, so `Var.SExtBits` is already counted in `R.getBitWidth()` (which is not the same as on line 1232) 


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1235
+            R = R.zeroExtend(R.getBitWidth() + Var.ZExtBits);
+          VarIndexRange = R.sextOrTrunc(Var.Scale.getBitWidth())
+                              .multiply(ConstantRange(Var.Scale));
----------------
nikic wrote:
> We probably don't need this sextOrTrunc() anymore?
We still need to extend from 32 to 64 bits when `--basic-aa-force-at-least-64b=0`. This is apparently not taken into account in `SExtBits`/`ZExtBits`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109746