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

[Sanjay Patel via llvm-commits]

spatel added subscribers: joker.eph, bkramer.
spatel added a comment.

In http://reviews.llvm.org/D16836#342693, @flyingforyou wrote:

> After investigating more, Select optimization only consider just one instruction. So without this patch, we do transform, even if we don't have sinking instruction.
>  I want to listen from Sanjay who made this change first.


Hi Junmo -

You're correct that the select optimization only considers one expensive instruction rather than following a chain. This was a hack to get the bug ( https://llvm.org/bugs/show_bug.cgi?id=24818 ) fixed quickly. Expanding that transform to look at more than one instruction was mentioned as a follow-on here:
http://reviews.llvm.org/D13297#257684

It was my mistake to not include a 'TODO' comment in the code about that.

I agree with Hal's point: the sinking of an expensive operand is not the converse of not sinking a cheap operand.

If I'm understanding this patch, it is bypassing the transform created by http://reviews.llvm.org/rL156234 and could regress whatever test motivated that patch.

After thinking about what that patch is trying to do and considering Mehdi's comments about hampering DAG transforms in http://reviews.llvm.org/D13297, I wonder if the correct solution is to redo r156234 at the machine-level (MachineCombiner pass?). I think that would solve the problem you're seeing. Ie, the problem is that the sinking transform is firing because of the *load* in your test case, but in your benchmarks, the loads are cheap, so we're causing a slowdown by removing selects.


http://reviews.llvm.org/D16836