[PATCH] D42600: [WIP][Shrink-wrap]split restore point

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added a comment.

Thanks for working on this! I think the idea is worth pursuing. This kinda reminds me of Post Register Allocation Spill Code Optimization, Christopher Lupo, Kent D. Wilken <https://dl.acm.org/citation.cfm?id=1122409>. A similar issue is what they describe in §4.8. I think if you extract the blocks C, D, E, F from the example, that's your function.

I tried looking into implementing that a while ago, and I mostly gave up because our SESE region infrastructure is not used / tested much, especially during CodeGen. I think your implementation has a big advantage because it doesn't require to build such (expensive, IIRC) constructions and might be much faster since SESE regions are usually not available, and dominators are.

Few thoughts: (please correct me if I'm wrong):

- We don't want to add extra branches. By that I mean that we want to guarantee that we always (or when we know it's not worth having extra branches) have a fall through from `NMBB` to `MBB`. In your example this is perfectly fine, and I see that `NMBB` is always inserted before `MBB`, which I think should be always fine.
- We want to be sure that, in the end, if the points are not interesting, we don't insert `NMBB`.
- I would run a verifier or add some statistics / remarks to see if any of the previous points are happening and if it causes any regressions.
- I think doing this kind of simulations on the post-dominator tree itself as @qcolombet suggested sounds interesting. Correct me if I'm wrong, here we're looking to introduce a common post-dominator of all the "dirty" blocks, right?

>   I observed 160% more shrink-wrapping in spec2000/2006/2017 benchmarks if we apply the copy forwarding (D41835), PostRASink (D41463), and this change all together.

Just to be sure, on AArch64, right? Do you see any performance improvement with this change? Any regressions?

Since you marked this as [WIP] I'll skip any comments on the code itself for now.


https://reviews.llvm.org/D42600