david-xl wrote: > > > > I don't understand, if you're saying the profile is accurate, then > > > > those functions are actually cold, so we should be able to mark them as > > > > optsize? > > > > > > > > > Accurate is not black or white. The current heuristic requires certain > > > level of accuracy to be effective. If you make the heuristics more > > > aggressive (like what this patch is doing), you're raising the > > > requirement of what can be considered accurate, and profile not meeting > > > that new requirement could see regression with new heuristic. > > > Whether a function is cold or not also depends on what is the calling > > > context and how inlining is done. All that makes function level > > > annotation inherently inaccurate when done before inlining. Not that we > > > shouldn't try it, but it's not as clear cut as it appears to be, and we > > > need to be careful. > > > > > > It will be more conservative (pre-inlining), so won't cause additional > > optimization suppression compared with the current PGSO. > > Sample PGO profile has inline context, so in the profile, we may have `foo` > as cold and `bar->foo` as hot, but if later inliner rejects `bar->foo` > inlining, `foo` can be hot. So marking `foo` as cold pre-inline can still be > inaccurate (and not conservative). > > In this PR, you have `PGOColdFuncAttr` default to `ColdFuncOpt::Default`. As > long as you keep it that way, this PR is fine for sample pgo (it's no-op > unless `pgo-cold-func-opt` is used explicitly).
Good example. This pass should be run post-inline. @aeubanks, any reason we want to run it early in the pipeline? https://github.com/llvm/llvm-project/pull/69030 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
