[PATCH] D48756: Add option for section ordering

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In https://reviews.llvm.org/D48756#1148415, @void wrote:

> Though frankly if the user doesn't specify `--segment-ordering-file`, then I'm not sure why the default is for this to be `true`.


Because `-function-sections` and `-data-sections` allow for code size optimizations, specifically `--gc-sections` and `--icf`.

In https://reviews.llvm.org/D48756#1149769, @davide wrote:

> In https://reviews.llvm.org/D48756#1149470, @tejohnson wrote:
>
> > This version is ok with me, question below around whether we want 2 separate options here though.
> >
> > Davide, did you consider changing the default for the clang driver options instead of just in gold/lld for LTO? It might also be nice to hook up -fno-function-sections and -fno-data-sections to this new gold option (and mirror it in lld), just as I had done for the affirmative versions of those options in tools::AddGoldPlugin (which is now essentially a no-op due to the default change). In which case we would want 2 separate options here (one for functions and one for data). Thoughts?
>
>
> Yeah, I think it's reasonable, so, no objections from me, but I'll let Peter decide as he owns LTO.


I don't think we should do that because it means that there are three possibilities in the clang driver:

- no flag: no function sections when compiling, function sections when linking
- `-ffunction-sections`: function sections when compiling, function sections when linking
- `-fno-function-sections`: no function sections when compiling, no function sections when linking

which would be a little confusing.

Also, without further information about the issue with the Linux kernel I don't see a reason why we should let users disable function sections because as far as I know it is just working around a bug. I would grudgingly accept it as a developer-only flag which allows developers to debug issues more easily, although to be honest if I were debugging such an issue I would do it by modifying the LTO code directly.



================
Comment at: tools/gold/gold-plugin.cpp:841
 
-  // Enable function/data sections by default.
-  Conf.Options.FunctionSections = true;
-  Conf.Options.DataSections = true;
+  Conf.Options.FunctionSections = !options::disable_section_ordering;
+  Conf.Options.DataSections = !options::disable_section_ordering;
----------------
Instead of adding a new flag, can you just make it so that these flags are sensitive to the existing `-function-sections` and `-data-sections` flags? You can do that like this:
```
if (FunctionSections.getNumOccurrences() == 0)
  Conf.Options.FunctionSections = true;
```
With that you can disable function sections with `-plugin-opt -function-sections=0`.


Repository:
  rL LLVM

https://reviews.llvm.org/D48756