When profile data is given, .hot/.unlikely section prefix is
added to hot/cold functions for linker to improve code locality. GCC
controls this behavior with '-f(no)-reorder-functions' flag, while LLVM
uses opt tool option '-profile-guided-section-prefix=true/false'. This
patch is for LLVM to support the same driver flag with GCC. LLVM side
patch is D34795.
Details
Diff Detail
- Repository
- rC Clang
- Build Status
Buildable 23940 Build 23939: arc lint + arc unit
Event Timeline
Update documentation. Please let me know if I need to update other documents as well. Thanks!
The patch itself is fine. The meta question is whether we expect this option to be generally useful?
Friendly ping. @davidxl, I think there's no harm to make clang consistent with gcc for compiler options, and I wonder if you have any concerns that I may miss. Thanks!
docs/ClangCommandLineReference.rst | ||
---|---|---|
1955 | prefix or suffix? Or just leave the details out (also consider the interaction with -ffunction-sections)? Consider documenting:
|
Rebase. Sorry I somehow missed the recent comments. I addresses @davidxl's comment on documentation. Thanks!
Excuse me for bring this up so late, but why do we want to make any such promises? As in: fundamentally, LLVM IR doesn't have any order property on the module level. I have yet so seen reasonable code where the order of functions matters for anything but performance. I've seen a few things that required -funit-at-a-time, most noticable GCC's CRT implementation. But those are all major hacks. So under what premise is it useful to have to even pretend to honor source code order?
@joerg Sorry but I'm not sure if I understand your question. This doesn't pretend to honor source code order, but makes linker to place "hot" functions under .text.hot section (There's no guarantee of ordering between functions inside .hot.text section) while "cold" functions under .text.unlikely section. This is purely for performance.
test/Driver/function-sections.c | ||
---|---|---|
77 | There should ideally be a test for the default behavior as well (and please consider mentioning the default behavior in the commit message). |
docs/ClangCommandLineReference.rst | ||
---|---|---|
1953 | Isn't this autogenerated from include/clang/Driver/Options.td? |
Isn't this autogenerated from include/clang/Driver/Options.td?