modimo added inline comments.
================ Comment at: llvm/lib/Passes/PassBuilder.cpp:522 FPM.addPass(createFunctionToLoopPassAdaptor( - std::move(LPM2), /*UseMemorySSA=*/false, DebugLogging)); + std::move(LPM2), /*UseMemorySSA=*/false, /*UseBlockFrequencyInfo=*/true, + DebugLogging)); ---------------- asbirlea wrote: > It doesn't look like `UseBlockFrequencyInfo` is used for LPM2 here and below. > Would it make sense to set it to `false` at this point? > Good catch, changed to false for LPM2. ================ Comment at: llvm/test/Other/pass-pipelines.ll:57 ; CHECK-O2: Loop Pass Manager -; CHECK-O2-NOT: Manager +; Requiring block frequency for LICM will place ICM and rotation under separate Loop Pass Manager ; FIXME: We shouldn't be pulling out to simplify-cfg and instcombine and ---------------- asbirlea wrote: > Add `; CHECK-O2: Loop Pass Manager` along with this comment. > > > Please consider if splitting the loop pass pipeline has any effects on > optimizations. This is for the legacyPM only, so those who switched to the > newPM will not be affected. > The solution may be to mark the analyses preserved in loop unswitch. To check my understanding here, with the split loop pass pipeline the phases look like the following: ``` Lazy Branch Probability Analysis Lazy Block Frequency Analysis Loop Pass Manager Loop Invariant Code Motion Loop Pass Manager Unswitch loops ``` Walking through the code of Loop Pass Manager by itself it doesn't re-calculate or produce additional analysis. Thus the difference appears to arise as follows: 1. combined loop pass: loop opts are run one after the other per loop, so if you have loops L1 and L2 the order would be L1(LICM, Unswitch) -> L2 (LICM, Unswitch) 2. split loop pass: in this case it would be L1(LICM)->L2(LICM) into L1(Unswitch)->L2(Unswitch) My qualitative assessment is that the impact here is quite minimal. The main scenario I can think of where differences could occur is having all LICM completed early can change the costing in unswitching for nested loops. Unswitching only occurs once in the legacyPM and always after LICM so it seems fairly tame to build in this cross-pass dependence by marking lazy BFI/BPI preserved. I'd like to know if this level of cross-pass dependence is potentially an issue and if there's precedence for doing so if you have that context. I've made the changes in the latest commit so that they exist in the same pass pipeline again by marking lazy BFI/FPI as preserved in unswitching. The value is definitely there to prevent unwanted side-effects. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
