[PATCH] D110246: [InstSimplify][InstCombine] Fold ptrtoint(gep i8 null, x) -> x

[Roman Lebedev via Phabricator via llvm-commits]

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

LG with nits.
No idea about non-integral pointers.



================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4685-4686
+        Value *LastIndexOp = std::prev(GEP->idx_end())->get();
+        bool OtherIndicesAllZero = all_of(
+            make_range(GEP->idx_begin(), std::prev(GEP->idx_end())),
+            [](const Use &Index) { return match(Index.get(), m_ZeroInt()); });
----------------
Then you might aswell just use the one from C++ STL taking separate begin/end


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4688
+            [](const Use &Index) { return match(Index.get(), m_ZeroInt()); });
+        if (LastIndexOp->getType() == Ty && OtherIndicesAllZero) {
+          // We also can't perform the fold if we need a type size multiplier.
----------------
The `LastIndexOp->getType() == Ty` check can be done earlier, before `all_of` check
(or just inline `OtherIndicesAllZero` variable?)


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4689-4692
+          // We also can't perform the fold if we need a type size multiplier.
+          // More complex cases are handled in InstCombineCompares.cpp.
+          TypeSize Stride = Q.DL.getTypeAllocSize(GEP->getResultElementType());
+          if (!Stride.isScalable() && Stride == 1)
----------------
Again, this seems like a cheaper check that should be done before `all_of`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110246