[PATCH] D67318: [SimplifyCFG] FoldTwoEntryPHINode(): consider *total* speculation cost, not per-BB cost

[James Molloy via Phabricator via llvm-commits]

jmolloy accepted this revision.
jmolloy added a comment.
This revision is now accepted and ready to land.

I can think of a rationale for the current behaviour. A lopsided if-triangle could indicate it might be better to emit a branch than speculate. Especially as this transform is context-free, input code that contains the same pattern many times can cause an explosion in code size and the transform was likely designed to be conservative.

Also note that there exist targets that don't use conditional moves to perform predication (most VLIW targets have a dedicated predicate register file) so that changes the behaviour somewhat.

I don't think there's a right answer for SimplifyCFG. Whatever we do is wrong, but sometimes it is useful. I think your approach here has been great; you have a bunch of performance data showing  improvements and no regressions. I also like your renaming from "Cost" to "Budget". That's annoyed me for a while.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67318