[PATCH] D68408: [InstCombine] Negator - sink sinkable negations

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D68408#1992227 <https://reviews.llvm.org/D68408#1992227>, @spatel wrote:

> In D68408#1991910 <https://reviews.llvm.org/D68408#1991910>, @nikic wrote:
>
> > Compile-time numbers look good: http://llvm-compile-time-tracker.com/compare.php?from=f52e0507574b4fd84dc4674536f5dfbab396c0f6&to=0a009b654793dee8e335c053eb043e297071e0d1&stat=instructions
> >
> > Change does not seem to have cost above noise, apart from a -0.6% improvement on tramp3d-v4. There's a corresponding -0.74% reducing in code-size, so this transform is clearly doing (or enabling) something big there.
> >  It might be interesting (but not necessary) to take a quick look at what happens there.
>
>
> Yes, it would be good to derive a regression test from that benchmark


F11767681: old.ll.xz <https://reviews.llvm.org/F11767681> F11767683: new.ll.xz <https://reviews.llvm.org/F11767683>
The impact there is quite noticeable as per llvm-diff, which almost immediately crashes.
It appears, a lot more inlining happens (functions now-missing in `new.ll`),
and some more function 'specialization' (functions now-appearing in `new.ll`).
I'm not sure i can distill/filter that to make any reasonable test case..

In D68408#1992227 <https://reviews.llvm.org/D68408#1992227>, @spatel wrote:

> and/or invent some larger tests that show the greater optimization power of the new code.
>  Unless I missed it, all of the current test diffs show that we do no harm, but if we can show that the added code/complexity buys us something immediately, that makes the benefit clear.


The claim i'm making in the patch's description is that we almost consider `sub` instruction
non-canonical, and we should be trying to fold it away as much as possible.
These test cases show the common patters, that we miss currently, where we can get rid of it.

This approach is what was requested in

In D68408#1695357 <https://reviews.llvm.org/D68408#1695357>, @efriedma wrote:

> > This *is* unusual. Is this too ugly to live?
>
> I'd prefer to actually transform the operation tree at the point we decide it's profitable, to make it clear we can't end up in an infinite loop or something like that.  As it is, you're depending on some other transforms happening in a particular order, and it's not clear that will happen consistently.  (Yes, it's a little more code, but I think that's okay.)



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68408/new/

https://reviews.llvm.org/D68408