[PATCH] D150864: [AggressiveInstCombine] Handle the nested GEP/BitCast scenario in Load Merge.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp:796
   Instruction *Inst = dyn_cast<Instruction>(LI1->getPointerOperand());
+  if (Inst && (isa<BitCastInst>((Inst)->getOperand(0)) || isa<GetElementPtrInst>((Inst)->getOperand(0))))
+    return false;
----------------
efriedma wrote:
> Are bitcasts and GEPs the only possibilities here?  stripAndAccumulateConstantOffsets can also handle addrspacecast and call instructions (and maybe other things in the future).
Right, this check doesn't looks right. It does not cover all possible instructions, and it will also fail spuriously if the root pointer is a (non-constant) GEP.

I'm having a hard time following what the code here is actually trying to do, but possibly what it should be doing is to create a new GEP with the correct offset, instead of trying to reuse an existing GEP chain, and thus avoid the dominance problems that come with that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150864