[PATCH] D146622: [AggressiveInstCombine] folding load for constant global patterened arrays and structs by GEP indices

[Kohei Asano via Phabricator via llvm-commits]

khei4 added inline comments.


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:909
   unsigned BW = DL.getIndexTypeSizeInBits(PtrOp->getType());
-  // TODO: Determine stride based on GEPs.
-  APInt Stride(BW, 1);
+  std::optional<APInt> Stride;
   APInt ConstOffset(BW, 0);
----------------
nikic wrote:
> khei4 wrote:
> > nikic wrote:
> > > I think we can keep this non-optional. `Stride=1` is always valid as a fallback.
> > yeah, but the GCD calculation process seems good to be undefined on the first.
> > I'll assign 1 on the following Stride's "nullopt" checking!
> You are right that inside getStrideAndModOffsetOfGEP() we should start with `std::optional<APInt>`. My thinking it that we can use plain `APInt` outside it. What do you think about making `getStrideAndModOffsetOfGEP()` return `std::pair<APInt, APInt>` and then calling it like `auto [Stride, ConstOffset] = getStrideAndModOffsetOfGEP(PtrOp, DL)`? In that case, getStrideAndModOffsetOfGEP would return Stride=1, ConstOffset=0 for the case where it can't determine anything.
Sounds good! I'll apply;)


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

https://reviews.llvm.org/D146622