I need to setup access token because github has changed commit policy recently.

It seems, I'm no longer having access to the llvm-project. Please commit this for me.

rename

ok i'll rename this before commit.

Rebase, fix a typo, change default hotcoldsplit-max-params=4

In D88460#2458205, @anjankgk wrote:

In D88460#2342937, @hiraditya wrote:

It'll be nice to know if this patch worked fine with a set of regression tests.
results on LLVM testsuite, or clang bootstrap will really help.

Thanks for working on this.

Thank you for the feedback @hiraditya! I have tested the latest patch at various options (O2, O3, LTO and PGO) against the SPEC benchmarks as well as run the lnt tests and bootstrap tests successfully. If you were concerned about any other specific testsuite, pls let me know.

the following failures reported by buildbot are unrelated to this change:

remove consume

Rebase

Maybe ask in Discord to get reviewers?

LGTM, an example of a gep with inbounds wouldbe nice. there are some test cases like: CodeGen/2009-02-13-zerosize-union-field.c, CodeGen/2010-07-14-ref-off-end.c etc from where we can take a reduced example for the doc.

@vsk Let me know if you dont have bandwidth to land. I can land it otherwise.

Please fix clang-format issues.

Instcombine also expands small (16 bytes, hardcoded) memcpy's

or if you prefer, I can land it in its current state.

Good to have these tests. I'd wait for the previous patches in the stack to be reviewed.

@rjf do you have numbers on Firefox. Let us know.

It'll be nice to know if this patch worked fine with a set of regression tests.
results on LLVM testsuite, or clang bootstrap will really help.

Any plans to merge this patch?

nit: If FIXME's are mostly future works, then please replace them with TODOs.

Is the test failure related to this patch?

Self approving because this is NFC change to outline function definitions.

rebase

then it may be possible to construct a synthetic/fake profile as a pre-processing step

Rebase

Addressed fhahn's comments

please run clang-format

49a944af7f1980b54d8a2be0ef640f5a956bc423

FWIW C++ doesn't really have a lattice anymore: https://wg21.link/p0418

Is there a guidance for the users of 'isStrongerThan', isStrongerThanUnordered etc.?

Is this patch still useful now that D53514 has landed?

please run clang-format.

..anyway I can try to describe how thing work using some example. Is this what you are looking for?

yes. Thank you.

Can we add more test cases to include eh_pad, invokeinst,

please run clang-format on the patch.

Overall approach looks good to me even when we don't see good SPEC-17 numbers as the optimization is intended to reduce page faults. The improvements would be more pronounced in large applications. I'll review the code in more detail in the next few days. Thanks for working on this.

Is this work based on any paper/implementation?
Can we add some documentation at the top of the file to get an overall idea of the cost model?

Fixed the bug in: https://bugs.llvm.org/show_bug.cgi?id=45442

Updated test cases.

FYI: libstdc++ patch sent for review: https://gcc.gnu.org/pipermail/gcc-patches/2020-August/551934.html

Self approving as it is a trivial change and fixes PR45442

In D85628#2214119, @jfb wrote:

In D85628#2213940, @rjf wrote:

In 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?

The patch looks legit. Wonder if such a patch should be sent to libstdc++ too.