[PATCH] D52671: [InstCombine] Skip merging non-free GEP

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1681
+    if (isa<Instruction>(Src) && !Src->hasOneUse() &&
+        TTI.getUserCost(Src) != TargetTransformInfo::TCC_Free) {
+      BasicBlock *CurBB = GEP.getParent();
----------------
junbuml wrote:
> efriedma wrote:
> > For array GEPs, we have a check to try to make sure we aren't making a GEP more expensive (see EndsWithSequential etc.)  Maybe instead of adding a different kind of check, it would make more sense to extend the existing check to struct GEPs?
> Did you mean to move this whole check (line 1680~1687) into EndsWithSequential? 
No.

We currently have a check in the "if (EndsWithSequential)" which restricts GEP folding.  That doesn't apply to your testcase because the last index is a struct index.  Instead, it falls into the "else if (isa<Constant>(*GEP.idx_begin())" case, which doesn't have a profitability check.  My suggestion is to adapt the EndsWithSequential profitability check to also apply to struct GEPs.


https://reviews.llvm.org/D52671