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

[Nikita Popov via Phabricator via llvm-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1222
+      Optional<ConstantRange> VarIndexRange;
       if (DecompGEP1.VarIndices.size() == 1) {
+        // VarIndex = Scale*V.
----------------
The code added here doesn't really rely on the fact that there is only a single index -- it could be integrated in the GCD/AllNonNegative/AllNonPositive loop above instead and add up the ranges of all indices. I'm okay with landing it in this limited form first though.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1234
+          if (Var.ZExtBits)
+            R = R.zeroExtend(R.getBitWidth() + Var.ZExtBits);
+          VarIndexRange = R.sextOrTrunc(Var.Scale.getBitWidth())
----------------
It's possible for both SExtBits and ZExtBits to be non-zero, so this should be extending to `R.getBitWidth() + Var.SExtBits + Var.ZExtBits`.


================
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));
----------------
We probably don't need this sextOrTrunc() anymore?


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1269
+          return AliasResult::NoAlias;
+        }
+
----------------
nit: Braces are unnecessary here.


================
Comment at: llvm/test/Analysis/BasicAA/range.ll:6
+
+declare i32 @llvm.annotation.i32(i32, i8*, i8*, i32)
+
----------------
Seems to be unused?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109746