[PATCH] D125845: [InstCombine] Canonicalize GEP of GEP by swapping constant-indexed GEP to the back

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added a comment.

We also see regressions with this patch in internal benchmarks and at least in one case it seems like the lack of "inbounds" on the new GEPs is involved.
If I change so the GEPs are created like

  -      Value *NewSrc = Builder.CreateGEP(
  +      Value *NewSrc = Builder.CreateInBoundsGEP(
             GEP.getSourceElementType(), Src->getOperand(0),
             SmallVector<Value *>(GEP.indices()), Src->getName());
  -      GetElementPtrInst *NewGEP = GetElementPtrInst::Create(
  +      GetElementPtrInst *NewGEP = GetElementPtrInst::CreateInBounds(

at least some regression disappear.
I do see the comment saying this can't be done though...

  // Cannot guarantee inbounds after swapping because the non-const GEP can
  // have arbitrary sign.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125845