
<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>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.</div><div><br></div><div>With the DT we can probably “globalize” combiner pattern by checking for and generalizing all "hasOneUse()” instances in the source.</div><div><br></div><div>-Gerolf</div><div><br></div><br><div><div>On Sep 4, 2014, at 12:14 PM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><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>

</blockquote></div><br></body></html>