[PATCH] D48756: Add option for section ordering

[Bill Wendling via Phabricator via llvm-commits]

void added a comment.

In https://reviews.llvm.org/D48756#1149891, @pcc wrote:

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


Then it should be dependent upon those options and not set `true` for every run. Note that given linkers don't normally create new sections for each function, running LTO on the program unexpectedly changes the resulting output. This isn't good, despite the fact that 99.99% of people may not notice a difference.

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

And I haven't been given a good reason why these options being on by default is necessary. Code size optimization isn't good enough if it doesn't work for all programs. As we all know, Linux is a special beast when it comes to compilation and linking. They've coupled many of their design decisions to how GCC, GAS, and the GNU linker operate. In this particular case, using function sections breaks the kernel. It's not a kernel bug because being sensitive to how things are linked isn't a bug. (You can argue that it's not as robust as it could be, but that's not the same as saying it's a bug.) Therefore, it's enough to say that "this doesn't work for all programs" to warrant adding an option to disable this feature. I chose to disable it by default because it won't reverse existing behavior, thus surprising existing users. However, I still doubt the need for it to be turned on by default; I'm just not going to argue the point further.


Repository:
  rL LLVM

https://reviews.llvm.org/D48756