[PATCH] D102002: [PassManager] unify vector passes between regular and LTO pipelines

Mon May 10 08:28:24 PDT 2021

spatel added a comment.

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

> As far as i can tell, on that codepath, current ThinLTO build does not exhibit the same slowdown as does the plain build with this patch.

AFAICT, ThinLTO is running the same set of passes as the default pipeline during this part of the compile. Ie, it's only fullLTO that has diffs like LoopUnroll before SLP, etc.

================
Comment at: llvm/lib/Passes/PassBuilder.cpp:1263-1268
   FPM.addPass(SimplifyCFGPass(SimplifyCFGOptions()
                                   .forwardSwitchCondToPhi(true)
                                   .convertSwitchToLookupTable(true)
                                   .needCanonicalLoops(false)
                                   .hoistCommonInsts(true)
                                   .sinkCommonInsts(true)));
----------------
spatel wrote:
> vzakhari wrote:
> > The code did not look like this at the place where https://reviews.llvm.org/rGfefcb1f878c2dad435af604955661ca02a5302de inserted a call to `addVectorPasses(... , /* IsLTO */ true)`.  It was only `.hoistCommonInsts(true)` - was this an expected change in behavior?
> No, that was a mistake. Thank you for spotting it!
> AFAIK, there's no way to show that difference in a pass manager unit test, so that's why it slipped through.
> Did you notice that by inspection or benchmark regression? If the latter, it would be great if we can reduce a test for it.
> Let me know if I should revert or update the earlier commit.
I went ahead and reverted the earlier change since it was clearly not NFC as advertised.

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

https://reviews.llvm.org/D102002