[PATCH] D144445: [AggressiveInstCombine] folding load for constant global patterened arrays and structs

[Kohei Asano via Phabricator via llvm-commits]

khei4 added a comment.

now ready for review again



================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:870-872
+            APIntOps::GreatestCommonDivisor(
+                APInt(ConstOffset.getBitWidth(), Stride),
+                APInt(ConstOffset.getBitWidth(), IndexTypeSize.getZExtValue()))
----------------
are there any better way to bitcast APInt?


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:929
+
+  for (uint64_t ByteOffset = Stride, E = GVSize - LoadSize; ByteOffset <= E;
+       ByteOffset += Stride)
----------------
khei4 wrote:
> nikic wrote:
> > Do we need the `LoadSize` adjustment here? Can we instead iterate to `ByteOffset < GVSize`?
> > 
> > The current `LoadSize` calculation is not right for pointers, but it seem like we can just drop it entirely.
> > Do we need the LoadSize adjustment here? Can we instead iterate to ByteOffset < GVSize?
> 
> I think bound-crossing load cannot be excluded, without LoadSize.
> like https://alive2.llvm.org/ce/z/jYsLBk
> Sorry, I'm not confident with the semantics of this. I'll try to ask in discord.
I removed loadSize in the end


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

https://reviews.llvm.org/D144445