[PATCH] D133567: [AA] Improve the BasicAA analysis capability

[Allen zhong via Phabricator via llvm-commits]

Allen marked 2 inline comments as done.
Allen added inline comments.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:597
       const Value *Index = *I;
+      TypeSize AllocaTypeSize = DL.getTypeAllocSize(GTI.getIndexedType());
       // Compute the (potentially symbolic) offset in bytes for this index.
----------------
nikic wrote:
> Please move this below the StructType check, this call is expensive.
Done, thanks


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:626
 
+      if (isa<ScalableVectorType>(GEPOp->getSourceElementType())) {
+        Decomposed.Base = V;
----------------
nikic wrote:
> Allen wrote:
> > nikic wrote:
> > > Here again we should be checking the alloc size, not the source element type. It should be possible to support something like `getelementptr <vscale x 4 x i32>, ptr %p, i64 0, i64 %i`, where the variable is on a non-scalable index.
> > Yes, good catch, thanks very much
> Can you please also add a test for this case? Something like using `%i` and `%j = add %i, 1` as indices. should probably work.
Of course, added new case redundant_load_elimination_zero_index_1


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

https://reviews.llvm.org/D133567