[PATCH] D113872: [CGP] Handle select instructions relying on the same condition

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a subscriber: lebedev.ri.
spatel added a comment.

In D113872#3240013 <https://reviews.llvm.org/D113872#3240013>, @lkail wrote:

>> could we prevent SimplifyCFG from forming these selects in the first place? Is the cost model broken in some way or just not accurate enough?
>
> Sorry I have missed this one. IIUC, `FoldTwoEntryPHINode` should be the place performing branch to select transformation. `FoldTwoEntryPHINode` checks
>
>   // Okay, we found that we can merge this two-entry phi node into a select.                                                                                                                  
>   // Doing so would require us to fold *all* two entry phi nodes in this block.                                                                                                               
>   // At some point this becomes non-profitable (particularly if the target                                                                                                                    
>   // doesn't support cmov's).  Only do this transformation if there are two or                                                                                                                
>   // fewer PHI nodes in this block.                                                                                                                                                           
>   unsigned NumPhis = 0;                                                                                                                                                                       
>   for (BasicBlock::iterator I = BB->begin(); isa<PHINode>(I); ++NumPhis, ++I)                                                                                                                 
>     if (NumPhis > 2)                                                                                                                                                                          
>       return false;
>
> which looks not accurate enough.
>
> Is it appropriate to improve this heuristic rather than in CGP, since SimplifyCFGPass looks like a canonical pass to me and also we lack subtarget info(which includes mispredict penalty) in SimplifyCFGPass?

cc @lebedev.ri who has also made adjustments on enabling the phi->select transforms in SimplifyCFG.

If we can adjust SimplifyCFG in some way to do the right thing for your motivating case and not break anything else, that would be great. :)
The SimplifyCFG transform is predicated on a mix of heuristics (like the hard limit of 2 phis) and costs (grep for "TTI.getUserCost(I, TargetTransformInfo::TCK_SizeAndLatency)"). 
If we can rely more on the cost model and less on the hard limits, that seems like a win. 
We will never get it completely right for everyone though because the cost modeling in IR will never be accurate enough (profile data should help, but even that may not capture enough info to get the ideal CFG for all targets).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113872