[PATCH] D127726: [InstCombiner] Add option to replace PHI of GEPs with GEP with PHI as index

[Elena Lepilkina via Phabricator via llvm-commits]

eklepilkina added a comment.

> We already have an implementation of this general transform -- shouldn't it be sufficient to relax only this condition? https://github.com/llvm/llvm-project/blob/7dc18a62e40e241019ec77e70f01bc41d39ab748/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp#L537 (At least for the basic case.)

Yes, I saw this code and make several experiments with it. But decided to add extra method. Let me try to describe my motivation.

1. Orginal fold methods use the approach that they check the first PHI operand. And work with cases when there are only GEPSs in PHI node. We need the case when there can be both GEP instructions and just array.
2. My experiments show that really in many coditions such modification makes worse (as mentioned in comment for condition you have provided a link for). If you looked at code we have added several conditions that are connected with uses of his PHI node and number of operands. As far I had to add all these not very obvious heuristics in this modification I decided to do it as separate method. Moreover, then it is easier to turn on/off it with flag. If integrate flag  and all needed heuristics inside exsiting method, it becomes not so readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127726