[PATCH] D146622: [AggressiveInstCombine] folding load for constant global patterened arrays and structs by GEP indices
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun May 7 04:09:35 PDT 2023
nikic added inline comments.
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:831-844
+ if (isa<GlobalVariable>(PtrOp)) {
+ ModOffset = APInt(BW, 0);
+ Stride = APInt(BW, 1);
+ return true;
+ }
+
+ // If PtrOp is neither GlobalVariable nor GEP, we cannot use GEP stride,
----------------
I think these two ifs are redundant with the `!isa<GlobalVariable>(PtrOpV)` check that's done below the while loop. It should be possible to just drop them.
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:854
+
+ for (auto [V, IndexTypeSize] : VarOffsets) {
+ APInt Scale = APInt(BW, IndexTypeSize.getZExtValue());
----------------
This looks confused. The second element in the pair is not the IndexTypeSize, but already the Scale (as an APInt).
================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:878
+ // In consideration of signed GEP indices, non-negligible offset become
+ // remainder of divission by minimum GEP stride.
+ ModOffset = ModOffset.srem(*Stride);
----------------
divission -> division
================
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);
----------------
I think we can keep this non-optional. `Stride=1` is always valid as a fallback.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146622/new/
https://reviews.llvm.org/D146622
More information about the llvm-commits
mailing list