<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 4, 2014 at 11:20 AM, Gerolf Hoflehner <span dir="ltr"><<a href="mailto:ghoflehner@apple.com" target="_blank">ghoflehner@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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.</blockquote><div><br></div><div>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.</div><div><br></div><div>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...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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.</blockquote><div><br></div><div>This shouldn't be too bad. There are other places where we combine somewhat specific uses or instruction patterns.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I also looked at the SimplifyCFG and if-conversion code, but none seem a natural fit for this optimization.</blockquote><div><br></div><div>I agree here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I’m not super happy about a new pass but it looked like the kleinstes Uebel among the choices.</blockquote></div><br>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.</div></div>