[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

Thu May 4 09:19:00 PDT 2023

aeubanks added a comment.

In D149800#4317275 <https://reviews.llvm.org/D149800#4317275>, @mtrofin wrote:

> high level question, why not have it as a pass that runs after profiles (of whatever kind - instrumented or sample-based) are ingested. The pass would attribute functions as described.

that was also my initial thought, but I found it weird that only the ifdo pass marked functions as cold, but not the samplepgo pass. I remembered something about "precise" profiles and thought maybe that had something to do with not marking functions as cold with samplepgo? But maybe `PSI->isFunctionColdInCallGraph(F, *BFI)` takes care of all of that

In D149800#4317483 <https://reviews.llvm.org/D149800#4317483>, @tejohnson wrote:

> Previously @yamauchi did a bunch of work related to this called PGSO (Profile Guided Size Optimization). See the functions in https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Utils/SizeOpts.cpp, which are used to guide a number of optimizations that can affect code size. Do you know why those weren't sufficient?

I think there are still a lot of places that just look at optsize/minsize. I'll take a look at the inline cost model more to see how it interacts with hotness. Separately, it seems good for compile times to calculate once if a function is cold or not and save that result as optsize/minsize.

> @davidxl may remember the details, but iirc fully using minsize/optnone for cold funcs might have been too big of a hammer performance-wise.

s/optnone/optsize? But I'm surprised that even minsize would make a perf difference just in functions that are supposedly cold. IIUC there aren't too many differences at least in the optimization pipeline for minsize functions aside from inlining thresholds (maybe there's more drastic changes in the backend?). It would be good to know if previous attempts at this sort of thing have shown perf regressions.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149800/new/

https://reviews.llvm.org/D149800