[PATCH] D101468: [Passes] Run sinking/hoisting in SimplifyCFG earlier.

Wed Apr 28 10:34:37 PDT 2021

lebedev.ri added a comment.

In D101468#2723273 <https://reviews.llvm.org/D101468#2723273>, @nikic wrote:

> In D101468#2723229 <https://reviews.llvm.org/D101468#2723229>, @lebedev.ri wrote:
>
>> In D101468#2723180 <https://reviews.llvm.org/D101468#2723180>, @nikic wrote:
>>
>>> Thanks! I like this a lot more than the LoopVectorize variant. I also think it makes sense that this happens at the end of the "function simplification" pipeline, which means that the inliner will see the hoisted/sunk IR, which should result in a more accurate cost (as sinkable/hoistable instructions will not be counted multiple times).
>>
>> Nonono, this is actually the opposite from what should happen.
>> We will then over-inline, and grow the size even more.
>
> I don't follow. It makes the inlining cost more accurate, so if that results in over-inlining, then the issue would be with the inlining cost model, not this change. It is my understanding that the inliner should see functions in their maximally simplified form (and prior to size-increasing optimizations like runtime unrolling and vectorization), so that it can make the most accurate decisions. Intentionally crippling the function simplification pipeline to get less inlining seems rather backward. Taken ad absurdum, that would mean that we shouldn't simplify functions prior to inlining at all.
>
>> This is even visible in the http://llvm-compile-time-tracker.com/compare.php?from=2ea7fb7b1c045a7d60fcccf3df3ebb26aa3699e5&to=e58b4a763c691da651f25996aad619cb3d946faf&stat=size-total
>
> This looks pretty normal for a phase ordering change. SPASS on `O3` is up 0.46%, lencod on `ThinLTO` is down 0.66%. The geomeans are 0.1% up or down for NewPM.

I see. So i take it, D101231 <https://reviews.llvm.org/D101231>, if restricted to function-terminating block, makes more sense to you now, in general?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101468