[PATCH] D85628: [HotColdSplitting] Add command line options for supplying cold function names via user input.

Wed Aug 12 22:24:56 PDT 2020

hiraditya added a comment.

In D85628#2214614 <https://reviews.llvm.org/D85628#2214614>, @yfeldblum wrote:

> In D85628#2214536 <https://reviews.llvm.org/D85628#2214536>, @rjf wrote:
>
>> I think you missed Aditya's point here: he's saying that certain things like outlining `__cxa*` functions might be unwise to mark as cold in general
>
> Maybe I missed that, or maybe I am missing some context. I didn't think there was a top-line goal of injecting cold attributes to arbitrary functions.
>
> My understanding is that a key motivating case is minimizing native codegen for local static variables by understanding that the slow path is definitionally cold. If coldness propagates, then as long as `__cxa_guard_acquire` is attributed as cold, coldness propagation should extend to everywhere of interest. And since `__cxa_guard_acquire` is used for only one purpose, namely guarding local static variable initialization, and since that purpose is definitionally cold, it should be attributed as cold. I didn't think there was a need to inject cold attributes to other functions.
>
> When it comes to library functions which are cold, presumably we can just patch libraries with cold-attributed function definitions but unattributed declarations, and attribute the declarations.

Added a patch to libcxxabi: https://reviews.llvm.org/D85873

> It seems like adding compiler support for lists of cold functions is a workaround for not patching libraries?

I think, in general, it would be useful to have a way to tell compiler the functions which are cold. This will avoid annotations, as it is possible to have a function hot in one context and cold in another e.g., inlining of std::string::string() which Laxman and I added in https://reviews.llvm.org/D22782 Although the patch improved some of the workloads quite significantly, it is also a constant source of noise to many other workloads.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85628