[PATCH] D16836: [CodeGenPrepare] Don't transform select instructions into branches when both of operands are cheap

Tue Feb 16 08:19:43 PST 2016

spatel added a comment.

In http://reviews.llvm.org/D16836#353403, @bkramer wrote:

> In http://reviews.llvm.org/D16836#345074, @spatel wrote:
>
> > I would be very interested to know the perf results of completely removing the heuristic-based load transform. I'm not sure what LLVM policy is for this type of situation. I've cc'd @bkramer in case it is possible to know which app/benchmark was improved by r156234. Clearly, it was driven by an improvement for OoO x86-64.
>
>
> Back then this had significant effects on the LLVM test suite on 2012-class x86_64 hardware. The heuristic is extremely dumb though because LLVM was/is lacking infrastructure to do it properly.
>
> 1. Introducing branches at the SelectionDAG level is extremely messy.
> 2. No profile info available (not even heuristically) on selects.
> 3. No info on the cost of an instruction. This is somewhat solved by TTI but I'm not sure if it handles loads.
>
>   If I were to implement it properly I would start by adding branch weights to selects, we currently lose a lot of profile data in SimplifyCFG. Then we can drive the transform by transforming biased selects into branches first, maybe with a TTI-based version of the current heuristic as a fall back if no info is there.
>
>   If you don't see any regressions from disabling the load-based heuristic feel free to throw it out. Both the hardware and LLVM have changed since 2012 so maybe the problems I was papering over with the load-based heuristic aren't there anymore.

Thank you for the background! I'll do some perf testing on AMD Jaguar (2013-class small core) with the current test-suite and report back.

I filed the branch weight metadata propagation enhancement here:
https://llvm.org/bugs/show_bug.cgi?id=26636

http://reviews.llvm.org/D16836