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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1234
+          if (Var.ZExtBits)
+            R = R.zeroExtend(R.getBitWidth() + Var.ZExtBits);
+          VarIndexRange = R.sextOrTrunc(Var.Scale.getBitWidth())
----------------
courbet wrote:
> 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) 
Oh right, that makes sense!


================
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));
----------------
courbet wrote:
> 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`.
The case where an extend is needed should be represented by SExtBits -- but the reverse case (where a truncate is needed) is currently not represented (which is an open bug). So yeah, you're right and we can't drop this entirely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109746