yuxuanchen1997 wrote: Hi @boomanaiden154, I missed your last comment but please allow me to address some of your concerns.
> I am not making an assumption about how users would use this. Not being able > to take into account the hotness of a callsite when making inlining decisions > will reduce performance compared to when you can take it into account. This patch is not taking away the ability of the compiler to determining inline appropriateness based on the hotness of a callsite. This patch in my opinion is focused on one thing, that is to extend an existing compiler functionality - [[gnu::flatten]] already designed to do. Attributes are powerful tools where performance and capacity engineers use as part of their day job to experiment with different setups, independent from AutoFDO. Highly specialized code paths like one in malloc and C++ standard library have a decent amount of such attributes to hint compiler to do different things based on empirical evidence. > `alwaysinline` is way more specific than this, which alleviates the problem. The fact that `[[*::alwaysinline]]` `[[*::noinline]]` overrides the PGO inlining decisions is a great testament to what these attributes are supposed to do. However, they won't exactly alleviate the problem without reasoning about transitive callees. > > Same goes for this extension, the effectiveness depends on how and where > > users apply it. Not using it correctly can cause worse performance is not a > > reason to not give that tool to users, otherwise alwaysinline would not > > exist either. One example, using this for memory allocator fast path when > > PGO isn't available (e.g. pre-builds) is one of those cases can benefit > > from it. > > > That said, I agree that using this to handle PGO stale profile may not be > > sustainable -- i won't do that. > > These two statements seem to be in conflict to me. It's also not clear to me > why adding `alwaysinline` to a couple functions within the allocator fast > path wouldn't be sufficient to solve the problem. I wouldn't say so. It would be an understatement to say it can be fulfilled by adding `alwaysinline` to "a couple functions". That motivated use of [[gnu::flatten]] which is not truly flattening. https://github.com/llvm/llvm-project/pull/165777 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
