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

Wed Aug 12 15:08:49 PDT 2020

jfb added a comment.

In D85628#2213940 <https://reviews.llvm.org/D85628#2213940>, @rjf wrote:

> In D85628#2213919 <https://reviews.llvm.org/D85628#2213919>, @vsk wrote:
>
>> I’m not convinced this is a good idea. In what use case is it not possible to mark up relevant functions? It doesn’t make sense to me to make alternations to standard library functions within the compiler. It seems better to simply patch the standard library. In some cases llvm does infer function attributes for library functions, but these are generally lower level attributes that can’t be specified at the source level, and the attribute is made available to other passes in the pipeline.
>
> Do you mean this patch isn't a good idea in general, or the recent revision isn't a good idea? For the latter, I'm not sure if you meant we should not outline declarations or we should not split the original loop into two (e.g. marking as cold before outlining). IMO splitting the loop into two simply addresses what the original intent of what we're doing, which is to mark certain functions as cold before outlining. Whereas, if we don't outline declarations via user-provided input, it renders @hiraditya 's proposed testcase useless. Alternatively, we don't have to make the testcase involving standard library functions if that's what you want :).

My understanding is that today code can be considered "cold" based on the following:

1. Attribute on the function
2. Likely / unlikely annotations
3. Profile information
4. Other compiler heuristics

This adds another way to do it, but it's kind of a side-injection and it doesn't seem particularly principled. Presumably the list you're feeding through the command-line comes from a profile? Why isn't it provided as profile information?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85628