WenleiHe wrote: > > > > Good example. This pass should be run post-inline. @aeubanks, any > > > > reason we want to run it early in the pipeline? > > > > > > > > > We want the main function simplification pipeline to see these function > > > attributes because some optimizations trigger or don't trigger depending > > > on the presence of the attributes. Modifying function attributes is > > > typically done in CGSCC/module passes since doing so can affect what > > > callers of those functions see (in effect changing other functions), > > > which shouldn't happen in function passes. I suppose it's possible to add > > > this as a CGSCC pass that runs after inlining and before the function > > > simplification pipeline, but this is more of a one time thing and CGSCC > > > passes can revisit functions. So this pass makes the most sense as a > > > module pass, but we can't insert a module pass between inlining and the > > > function simplification pipeline. > > > Can/does the inliner ignore these size attributes when it has call-site > > > profile information? > > > > > > Looking at the current change, this new pass is actually after the sample > > loader (including sample loader inlining) pass, so wenlei@'s concern should > > be addressed. > > Oh I forgot that the sample profile has its own inliner. Yes this pass runs > after we load profiling information since it uses profiling information, > whether it's sample or instrumented. > > Is this patch good to go?
This is good to go only because it's off by default. Otherwise it's not. Left one more comment on the patch as I didn't get a chance to look at final code before it went in. 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
