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

[Sanjay Patel via llvm-commits]

On Fri, Jan 13, 2017 at 5:24 AM, Volkan Keles <vkeles at apple.com> wrote:

>
> On Jan 12, 2017, at 4:35 PM, Sanjay Patel <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 - 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 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?


>
> 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.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170113/da9ecd5e/attachment.html>