zequanwu added inline comments.
================ Comment at: llvm/test/Other/opt-O2-pipeline.ll:289 +; CHECK-NEXT: Branch Probability Analysis +; CHECK-NEXT: Block Frequency Analysis ; CHECK-NEXT: FunctionPass Manager ---------------- MaskRay wrote: > zequanwu wrote: > > zequanwu wrote: > > > nikic wrote: > > > > hans wrote: > > > > > nikic wrote: > > > > > > Is it possible to switch this pass to use LazyBPI / LazyBFA, only > > > > > > fetched if PGO is actually in use? > > > > > > > > > > > > PGO functionality that most people don't use adding expensive > > > > > > analysis passes like PDT should be avoided. > > > > > I wonder if just switching to LazyBlockFrequencyInfo would help > > > > > though. It looks to me like the CGProfile would request info about > > > > > each function anyway. > > > > > > > > > > I was surprised to see that Clang sets Opts.CallGraphProfile solely > > > > > based on whether the integrated assembler is used. Maybe a better fix > > > > > is to only set that to true when a profile is actually being used? > > > > > I wonder if just switching to LazyBlockFrequencyInfo would help > > > > > though. It looks to me like the CGProfile would request info about > > > > > each function anyway. > > > > > > > > It would only help if there is some way to only fetch the analysis > > > > conditionally. I believe many PGO passes use something like > > > > PSI.hasProfileSummary() or F.hasProfileData() for that. > > > > > > > > > I was surprised to see that Clang sets Opts.CallGraphProfile solely > > > > > based on whether the integrated assembler is used. Maybe a better fix > > > > > is to only set that to true when a profile is actually being used? > > > > > > > > Right, just disabling this by default in clang/opt would also work. > > > > > > > > For reference, the current compile-time numbers for this patch: > > > > https://llvm-compile-time-tracker.com/compare.php?from=516ff1d4baee28b1911737e47b42973567adf8ff&to=8df840660bb764b6653fcfd9ac7a72cc6adebde6&stat=instructions > > > > Not huge, but it adds up (some similar regressions have been > > > > introduced in LLVM 10). > > > Do you mean disabling it just for LPM or both? > > > I was surprised to see that Clang sets Opts.CallGraphProfile solely based > > > on whether the integrated assembler is used. Maybe a better fix is to > > > only set that to true when a profile is actually being used? > > For Clang, a better fix I think is that `Opts.CallGraphProfile` should > > based on both whether the integrated assembler is used and whether profile > > instrumentation is turned on. What do you think? > I'd prefer not having `CallGraphProfile` > > * `-no-integrated-as -S` => no .cgprofile (.llvm_addrsig behaves this way) > * `-S` -> .cgprofile As discussed above, I think `CGProfilePass` should be disabled by default in clang unless `-no-integrated-as` is not given and `-fprofile-instrument-use-path=` is given. So, `Opts.CallGraphProfile` is a convenient switch for that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83013/new/ https://reviews.llvm.org/D83013 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
