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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

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)



================
Comment at: llvm/test/LTO/PowerPC/data-sections-linux.ll:9
+; RUN:   %t/no-data-sections.o
+; RUN: obj2yaml %t/default.o | FileCheck --match-full-lines %s
+; RUN: obj2yaml %t/data-sections.o | FileCheck --match-full-lines %s
----------------
Using obj2yaml for symbol attributes is overkill and is less readable.

Use `llvm-objdump -t`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129401