Page MenuHomePhabricator

upporting -f(no)-reorder-functions flag, clang side change
Needs ReviewPublic

Authored by twoh on Jun 28 2017, 6:10 PM.

Details

Summary

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.

Event Timeline

twoh created this revision.Jun 28 2017, 6:10 PM
twoh added a comment.Jul 31 2017, 8:50 AM

Ping. Thanks!

davidxl edited edge metadata.Jul 31 2017, 9:39 AM

The patch is missing documentation change.

twoh updated this revision to Diff 108967.Jul 31 2017, 12:01 PM

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?

twoh added a comment.Aug 5 2017, 11:47 AM

I think it is generally good to match what GCC does to not to confuse people.

twoh added a comment.Aug 16 2017, 10:40 AM

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!

espindola edited reviewers, added: espindola; removed: rafael.Mar 15 2018, 9:47 AM
davidxl added inline comments.Mar 15 2018, 9:54 AM
docs/ClangCommandLineReference.rst
1750 ↗(On Diff #108967)

prefix or suffix? Or just leave the details out (also consider the interaction with -ffunction-sections)?

Consider documenting:

  1. what is the default behavior?
  2. how does it affect function ordering?
twoh updated this revision to Diff 170148.Oct 18 2018, 11:39 PM

Rebase. Sorry I somehow missed the recent comments. I addresses @davidxl's comment on documentation. Thanks!

joerg added a subscriber: joerg.Oct 19 2018, 4:34 AM

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?

twoh added a comment.Oct 19 2018, 8:48 AM

@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.

twoh updated this revision to Diff 170205.Oct 19 2018, 8:49 AM

Remove conflict line.

srhines added inline comments.Dec 7 2018, 10:15 AM
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).

lebedev.ri added inline comments.
docs/ClangCommandLineReference.rst
1953 ↗(On Diff #170205)

Isn't this autogenerated from include/clang/Driver/Options.td?