[PATCH] D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline

Thu Aug 20 07:53:35 PDT 2020

echristo added a comment.

In D84108#2227701 <https://reviews.llvm.org/D84108#2227701>, @lebedev.ri wrote:

> In D84108#2227371 <https://reviews.llvm.org/D84108#2227371>, @echristo wrote:
>
>> One more follow up here:
>>
>> One set of benchmarks we're seeing this in are also in the llvm test harness:
>>
>> llvm_multisource_tsvc.TSVC-ControlFlow-flt and llvm_multisource_misc.Ptrdist-yacr2
>>
>> in an FDO mode (so we have some decent branch hints for the code).
>>
>> You should be able to duplicate at least some of the slowdown there.
>
> That sadly doesn't tell me much.
> Can you please provide the reproduction steps, and ideally the IR with/without this change?

It should be as straightforward as -O3 -fexperimental-new-pass-manager, if that isn't then we'll look a bit more. Those tests are in the llvm test-suite so they should be easy to get as part of your checkout.

Ideally for a change as large as this you'd have run these benchmarks yourself - they're fairly useful here and I think I'd have expected them if I were reviewing.

> In D84108#2227365 <https://reviews.llvm.org/D84108#2227365>, @echristo wrote:
>
>> So, we're seeing several significant (20-30%) regressions due to this in various different library benchmarks. Usually around things that are compression/decompression loops, but also other places.
>>
>> I'm uncertain what we can do, but I think this might need a wider range of discussion rather than the phabricator review.
>>
>> Would you mind terribly reverting and starting a thread on llvm-dev about this? I think that'll give us an opportunity to weigh some of the tradeoffs in a wider space.

I think I'm still here. I'll comment a bit more in reply to David below.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84108