[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

[Quinn Pham via Phabricator via cfe-commits]

quinnp added a comment.

In D129401#3666238 <https://reviews.llvm.org/D129401#3666238>, @MaskRay wrote:

> Mostly looks good, with a nit in the test and some suggestion to the summary.
>
> In D129401#3662857 <https://reviews.llvm.org/D129401#3662857>, @quinnp wrote:
>
>>> If this is for the legacy LTO interface, please state so.  `lld/*/LTO.cpp` sets `c.Options.DataSections = true;` to enable data sections by default.
>>
>> Hey @MaskRay, I'm not sure what is considered the legacy LTO interface, but this change is to make the `libLTO` codegen match the behaviour of `LTO` used through `lld` and `gold plugin`. Both `lld` and `gold plugin` turn on `data-sections` for `LTO` by default:
>>
>> - as you mentioned `lld/*/LTO.cpp` sets `c.Options.DataSections = true;` by default.
>> - and `llvm/tools/gold/gold-plugin.cpp` sets `Conf.Options.DataSections = SplitSections;` provided that the user did not explicitly set/unset `data-sections` where `SplitSections` is `true` unless `gold plugin` is doing a relocatable link.
>
> There is a legacy LTO interface (see llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h) and a resolution-based interface.
> Change libLTO in "This patch changes libLTO to set data-sections by default." to legacy LTO.
>
>> This patch also fixes the forwarding of the clang options -fno-data-sections and -fno-function-sections to libLTO
>
> This sentence can keep using libLTO or LLVMLTO (the library is LLVMLTO per llvm/lib/LTO/CMakeLists.txt)

Ah I see, thank you @MaskRay! I've updated the testcases and the summary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129401/new/

https://reviews.llvm.org/D129401