[PATCH] D139275: [SimplifyCFG] `FoldBranchToCommonDest()`: deal with mismatched IV's in PHI's in common successor block

Thu Jan 12 06:17:46 PST 2023

lebedev.ri added a comment.

@apilipenko @samtebbs please, can you clarify, do you wish to block relanding of this patch until those issues you saw are addressed, or at least reduced and reported?

In D139275#4046383 <https://reviews.llvm.org/D139275#4046383>, @alexfh wrote:

> In D139275#4022050 <https://reviews.llvm.org/D139275#4022050>, @lebedev.ri wrote:
>
>> In D139275#4022015 <https://reviews.llvm.org/D139275#4022015>, @apilipenko wrote:
>>
>>> FYI, this change caused widespread performance regressions in our testing. We haven't investigated further since the change was reverted.
>>
>>
>>
>>> I can re-run these tests and do some analysis on the next revision once you decide to reapply.
>>
>> At this point this is waiting on @alexfh's reproducer mostly.
>
> IIUC, the reproducer you mentioned is the one here: https://reviews.llvm.org/rG9ddff66d0c9c3e18d56e6b20aa26a2a8cdfb6d2b#1158810. Given that there was a bit of confusion and the problem had been fixed in ToT before the issue was reported <https://reviews.llvm.org/rG9ddff66d0c9c3e18d56e6b20aa26a2a8cdfb6d2b#1157049>, there's no concerns on our side about relanding this commit.
>
> Just fyi, our performance testing shows around 20% improvements of the mediabench-g721 benchmark from the LLVM's MultiSource test-suite associated with the reland of this patch (428f36401b1b695fd501ebfdc8773bed8ced8d4e <https://reviews.llvm.org/rG428f36401b1b695fd501ebfdc8773bed8ced8d4e>) and no regressions on any other benchmarks we run.

Thank you!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139275