[PATCH] Select elimination pass
ghoflehner at apple.com
Sun Sep 7 22:49:42 PDT 2014
Ok, I looked at InstCombine again and think I have a way now to catch this pattern without much ado when the DT is available. The key is recognize the pattern starting from the final icmp rather than the select. I will drop the new pass approach and post a new review.
With the DT we can probably “globalize” combiner pattern by checking for and generalizing all "hasOneUse()” instances in the source.
On Sep 4, 2014, at 12:14 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
> On Thu, Sep 4, 2014 at 11:20 AM, Gerolf Hoflehner <ghoflehner at apple.com> wrote:
> This is a good question. InstCombine is a local pattern matcher at the instruction level. I did not want adding a global flavor by introducing the dominator tree in that pass.
> This is an interesting point. Despite not having a dominator tree, there are parts of InstCombine that already approximate dominance information by scanning instructions or checking parent basic blocks.
> I think we should seriously consider adding DominatorTrees to InstCombine rather than adding more passes. I'm not aware of anything that would make this particularly hard or expensive...
> Also I would have to mess around with the combiner logic when inserting an instruction like an “or” which is not the immediate replacement instruction of the select.
> This shouldn't be too bad. There are other places where we combine somewhat specific uses or instruction patterns.
> I also looked at the SimplifyCFG and if-conversion code, but none seem a natural fit for this optimization.
> I agree here.
> I’m not super happy about a new pass but it looked like the kleinstes Uebel among the choices.
> Yea, I'm very unconvinced about adding a new pass here. Consider that this is quite likely to expose more instruction combining opportunities, or to be enabled by some other instruction combining. It really needs to be in the iterative worklist IMO.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits