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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:1263-1268
   FPM.addPass(SimplifyCFGPass(SimplifyCFGOptions()
                                   .forwardSwitchCondToPhi(true)
                                   .convertSwitchToLookupTable(true)
                                   .needCanonicalLoops(false)
                                   .hoistCommonInsts(true)
                                   .sinkCommonInsts(true)));
----------------
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.


================
Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:1091
 
   addExtensionsToPM(EP_Peephole, PM);
 
----------------
vzakhari wrote:
> There is another call to `addExtensionsToPM(EP_Peephole)` in `addVectorPasses` - is this expected?
No - that's another mistake that slipped by the unit tests. I can revert and/or fix. Please let me know which is preferred.


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

https://reviews.llvm.org/D102002