[PATCH] D16836: [CodeGenPrepare] Don't transform select instructions into branches when both of operands are cheap
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 3 08:14:32 PST 2016
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:
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.
More information about the llvm-commits