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

[Vyacheslav Zakharin via Phabricator via llvm-commits]

vzakhari 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)));
----------------
spatel wrote:
> 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.
I spotted it just during an inspection of a downstream conflict.  Sorry for the late reply, I do not think you had to revert it :)


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

https://reviews.llvm.org/D102002