[PATCH] D13297: [CGP] transform select instructions into branches and sink expensive operands

[Mehdi AMINI via llvm-commits]

joker.eph added a comment.

In http://reviews.llvm.org/D13297#257035, @spatel wrote:

> In http://reviews.llvm.org/D13297#256940, @joker.eph wrote:
>
> > knowing that SelectionDAG operates per basic-block, this makes me worried about implications on the ability of SelectionDAG to combine. I feel this is something that would better be done at the MI level.
>
>
> I've learned that the longstanding existence of something in LLVM does not make it right. :)


Definitively. This is not something "right" this is a limitation of SelectionDAG and I know people are actively working on getting rid of it.

> However, I want to note for the record that we have similar 'undo' transforms already in IR and/or DAG. These are the cases that I'm aware of:

> 

> 1. SimplifyCFG creates selects from branches using TTI (this behavior was toggled for some cases at r228826 and r248439).

> 2. SimplifyCFG combines branches with logic ops in FoldBranchToCommonDest() using a target-independent heuristic.

> 3. SelectionDAGBuilder may undo #2 in visitBr() using TLI.

> 4. CGP may undo #2 in splitBranchCondition() using TLI (and has a FIXME stating #3 should be removed).

> 5. CGP may undo #1 in optimizeSelectInst() using TLI (that's the code this patch is building on).

> 

>   I wonder if any of these raise the same concern? Again, I don't know what the right answer is...just asking a question. Also, note that this patch was proposed in http://reviews.llvm.org/D12882, and I thought there was some consensus for it. I may have misunderstood.


Indeed the "correct" approach may be to not "optimize" for the way SelectionDAG work and its limitation, and instead relying on MI-level combine pass to figure out later combine across basic blocks.


http://reviews.llvm.org/D13297