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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D67318#1671524 <https://reviews.llvm.org/D67318#1671524>, @xbolva00 wrote:

> Seems pretty ok: http://lnt.llvm.org/db_default/v4/nts/129108
>
> except "MultiSource/Benchmarks/FreeBench/fourinarow/fourinarow" really big perf/code size regression. Roman, can you look at it?


Hmm yeah, not surprised at all that it regressed.
F10002226: fourinarow.c <https://reviews.llvm.org/F10002226> F10002101: old.processed.ll <https://reviews.llvm.org/F10002101> F10002103: new.processed.ll <https://reviews.llvm.org/F10002103>
The patch did what it was supposed to do, and got rig of all the branching in `@value`.
And in that case it's bad because we end up with a wall (~1500 lines) of basically

  %tmp519 = and i64 %tmp62, %tmp233
  %tmp520 = icmp eq i64 %tmp519, %tmp62
  %tmp521 = sitofp i32 %tmp518 to float
  %tmp522 = fadd float %tmp236, %tmp521
  %tmp523 = fptosi float %tmp522 to i32
  %tmp524 = select i1 %tmp520, i32 %tmp523, i32 %tmp518

That test is really perfect storm for this. And i really don't like those int-float-int translations;
if i get rid of them (obviously not the fix), the regression goes away - even though in IR the flattening
still happens, in the asm we again get branches. So i suppose this may be a bug in undo transform in backend.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67318