MaskRay 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
----------------
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


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

Reply via email to