[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added a comment. In D86156#2245103 , @nikic wrote: > I have no familiarity with BFI, so possibly stupid question: There is already > some similar handling as part of BFIImpl here: >

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288719. modimo added a comment. Remove redundant VH callback as @nikic helpfully pointed out! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: llvm/include/llvm/Analysis/LoopAnalysisManager.h

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

nikic added a comment. Herald added a subscriber: danielkiss. New compile-time numbers: https://llvm-compile-time-tracker.com/compare.php?from=d7c119d89c5f6d0789cfd0a139c80e23912c0bb0=e0a1a6cac1b982023f8ceba8285d1ee7bc96bd32=instructions The regression is now reduced to 0.2%. I assume that's

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

wenlei added a comment. In D86156#2243393 , @asbirlea wrote: > Diff looks reasonable at this point. Thank you for the patch! > Please wait on @nikic for compile-time impact or additional feedback. > > Just out of curiosity, in D65060

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

asbirlea accepted this revision. asbirlea added a comment. Diff looks reasonable at this point. Thank you for the patch! Please wait on @nikic for compile-time impact or additional feedback. Just out of curiosity, in D65060 , it was mentioned that using BFI got

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added inline comments. Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:273 : nullptr; +BlockFrequencyInfo *BFI = UseBlockFrequencyInfo + ? ((F)) asbirlea wrote: > Add `&&

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288477. modimo added a comment. Condition usage of BFI to PGO in newPM as well CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: llvm/include/llvm/Analysis/BlockFrequencyInfo.h

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

asbirlea added inline comments. Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:273 : nullptr; +BlockFrequencyInfo *BFI = UseBlockFrequencyInfo + ? ((F)) Add `&&

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added a comment. In D86156#2242872 , @asbirlea wrote: > As a general note, it may make sense to include BFI in the set of loop passes > always preserved (`getLoopPassPreservedAnalyses()`), if its nature is to > always be preserved (with some

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288445. modimo added a comment. only use BFI when profile is enabled, have LICM mark BFI as preserved CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: clang/test/CodeGen/thinlto-distributed-newpm.ll

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

nikic added a comment. Current compile-time numbers show a 1% geomean regression: https://llvm-compile-time-tracker.com/compare.php?from=d7c119d89c5f6d0789cfd0a139c80e23912c0bb0=127615d90c7b4424ec83f5a8ab4256d08f7a8362=instructions I've left a comment inline with a possible cause.

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

asbirlea added a comment. Your understanding is correct. I only have one more comment on preserving passes in LICM too. As a general note, it may make sense to include BFI in the set of loop passes always preserved (`getLoopPassPreservedAnalyses()`), if its nature is to always be preserved

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

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, +

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288172. modimo added a comment. Remove usage need for BFI in LPM2 and set unswitching to preserve lazy BPI/BFI so it can remain in the same loop pass as LICM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

asbirlea 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, +

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added a comment. In D86156#2231710 , @nikic wrote: > This change adds three PDT calculations to the standard pipeline. Please try > to avoid the PDT calculations if PGO is not used, possibly by using LazyBPI. Good catch, changed to use LazyBFI

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 288125. modimo added a comment. Change to LazyBFI for legacy pass manager to prevent rebuilding the post-dominator tree CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files:

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

wenlei added a comment. > If only LICM benefits from it, I'd consider creating a BFI instance for the > duration of the LICM pass Currently only LICM uses BFI, but I think it'd be beneficial for other loop passes to be able to use BFI too. BFI not accessible to loop passes seems to be a

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

nikic requested changes to this revision. nikic added a comment. This revision now requires changes to proceed. This change adds three PDT calculations to the standard pipeline. Please try to avoid the PDT calculations if PGO is not used, possibly by using LazyBPI. CHANGES SINCE LAST ACTION

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo added inline comments. Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:408 FunctionToLoopPassAdaptor createFunctionToLoopPassAdaptor(LoopPassT Pass, bool UseMemorySSA = false, +bool UseBlockFrequencyInfo = false,

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 287152. modimo added a comment. Herald added subscribers: lxfind, nikic. @asbirlea Thanks for taking a look! I updated BFI to resemble MSSA as recommended which removed the BFI calculation unless LICM is invoked. Also removed the spurious diffs from

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

asbirlea requested changes to this revision. asbirlea added a comment. This revision now requires changes to proceed. A couple of quick comments, I haven't gone into details yet. - please split off the changes that are NFCs (deleted spaces) and clang-format for ease to review. - the test

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 286392. modimo added a comment. Commit my changes (crazy I know) so that the diff is actually updated for linting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files:

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo updated this revision to Diff 286390. modimo added a comment. Linting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86156/new/ https://reviews.llvm.org/D86156 Files: clang/test/CodeGen/thinlto-distributed-newpm.ll llvm/include/llvm/Analysis/BlockFrequencyInfo.h

[PATCH] D86156: [BFI] Preserve BFI information through loop passes via VH callbacks inside LoopStandardAnalysisResults

modimo created this revision. modimo added reviewers: wenlei, asbirlea, vsk. modimo added a project: LLVM. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, hiraditya. Herald added a project: clang. modimo requested review of this revision. D65060