[PATCH] D101536: [SimplifyCFG] Common code sinking: drop profitability check in presence of conditional predecessors (PR30244)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D101536#2742629 <https://reviews.llvm.org/D101536#2742629>, @xbolva00 wrote:

> Can you collect some performance stats?
>
> - testsuite/spec benchmarks/etc?
>
>>> Speculatively land and see if some other perf regression is reported against this change?
>
> Try to collect some data first

Thank you for stating your opinion.
As you can read in my previous comments, i have obviously already tried that locally.
test-suite is just hopelessly noisy, does that match with your expirience?
On the benchmark this does affect, the results are bizzare to me,
because there are *no* IR, or ASM level changes on the codepath being run,
yet there's +~10% run-time regression. Unless i'm slowly (rapidly) loosing it,
i can only attribute it to some weird code alignment issue
caused by the changes this did cause to other code.

In D101536#2742764 <https://reviews.llvm.org/D101536#2742764>, @davidxl wrote:

> please add more test cases to cover the expected new sinking cases enabled with this change.

The test covers it quite well - if we'll only sink already-speculatable instructions,
and not all predecessors are unconditional, we will now actually sink it.
So could you please be more specific what tests would you like to see?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101536