<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 13, 2017 at 5:24 AM, Volkan Keles <span dir="ltr"><<a href="mailto:vkeles@apple.com" target="_blank">vkeles@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;"><br><div><blockquote type="cite"><span><div>On Jan 12, 2017, at 4:35 PM, Sanjay Patel <<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>> wrote:</div><br class="gmail-m_2307541518420224187m_-2570252747655182579Apple-interchange-newline"></span><div><div dir="ltr"><div><div><div>Hi Volkan,<br><br></div><span>I reduced your test case and explained what I found below. It's not clear to me what you mean by linking the compile-time thread:<br></span></div><span>1. SCCP handles this case already, so we shouldn't need to add a duplicate fold to InstCombine and increase compile-time further?</span></div></div></div></blockquote><span><blockquote type="cite"><div><div dir="ltr">2. You are opposed to the min/max canonicalization in this commit because it is increasing compile-time?</div></div></blockquote><div><br></div></span><div>Sorry, my e-mail was not clear. I meant the former, I wanted to get your opinion.</div></div></div></blockquote><div><br><br></div><div>I understand the concern, but this commit (like the min/max commit - <a href="https://reviews.llvm.org/D27531">https://reviews.llvm.org/D27531</a> - that was inspired by this thread and referenced in the compile-time thread) is a very simple matcher. If these kinds of commits are causing noticeable slowdowns, then we need a structural fix...or LLVM is doing a lousy job of optimizing its own code - PR28430 is a bit embarrassing.<br><br>IMO, we need many more simple canonicalizations like this to make life easier for the backend. I've explained the rationale for the min/max additions (better codegen). This actually goes back to an earlier, more general suggestion on llvm-dev that we should canonicalize harder towards 'select' instructions in IR. We may want to rethink that strategy given the compile-time creep, but I'm definitely not knowledgeable enough to make that decision.<br><br>Several suggestions were made in the compile-time thread about improving instcombine, so I think that's where we should continue the discussion about compile-time in general.<br></div><div><br></div><div>I am curious about something here though. You noticed this difference in instsimplify behavior - does that mean that SCCP doesn't run in your optimization pipeline?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;"><div><span><blockquote type="cite"><div><div dir="ltr"><div><div><div><div><div class="gmail_extra"><br></div><div class="gmail_extra">As I said earlier, the canonicalization seems equally likely to enable more folding in InstSimplify as it does to bypass it. Do you agree?<br></div></div></div></div></div></div></div></blockquote><div><br></div></span>Yes, I agree.</div><div><br></div><div><span><blockquote type="cite"><div><div dir="ltr"><div><div><div><div><div class="gmail_extra"><br></div><div class="gmail_extra">My motivation for canonicalization of min/max patterns is that the backend is full of holes recognizing and lowering these patterns, so the more we can standardize them here in IR, the less we have to worry about botching the lowering. I have not attempted to weigh the compile-time cost of canonicalization against the potential runtime perf wins.<br></div></div></div></div></div></div></div></blockquote></span><br></div></div></blockquote></div></div></div>