[llvm] r287585 - [InstCombine] canonicalize min/max constant to select's false value

[Volkan Keles via llvm-commits]

> On Jan 13, 2017, at 3:16 PM, Sanjay Patel <spatel at rotateright.com> wrote:
> 
> 
> 
> On Fri, Jan 13, 2017 at 5:24 AM, Volkan Keles <vkeles at apple.com <mailto:vkeles at apple.com>> wrote:
> 
>> On Jan 12, 2017, at 4:35 PM, Sanjay Patel <spatel at rotateright.com <mailto:spatel at rotateright.com>> wrote:
>> 
>> Hi Volkan,
>> 
>> 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:
>> 1. SCCP handles this case already, so we shouldn't need to add a duplicate fold to InstCombine and increase compile-time further?
>> 2. You are opposed to the min/max canonicalization in this commit because it is increasing compile-time?
> 
> Sorry, my e-mail was not clear. I meant the former, I wanted to get your opinion.
> 
> 
> I understand the concern, but this commit (like the min/max commit - https://reviews.llvm.org/D27531 <https://reviews.llvm.org/D27531> - 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.
> 
> 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.
> 
> 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.

I’m not opposed to the min/max canonicalization and I’m not worried about its impact on compile-time, it was related to InstCombine. I wanted to know if that expensive call was going to be added to InstCombine. 

> 
> 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?

No, it doesn’t.

>  
>> 
>> As I said earlier, the canonicalization seems equally likely to enable more folding in InstSimplify as it does to bypass it. Do you agree?
> 
> Yes, I agree.
> 
>> 
>> 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.
> 

Volkan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170116/caf31ca8/attachment.html>