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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb6cc5ddc9478: [libLTO] Set data-sections by default in libLTO. (authored by quinnp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

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

quinnp updated this revision to Diff 448036. quinnp added a comment. Adding lit config to mark tests as unsupported for non PPC targets. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129401/new/ https://reviews.llvm.org/D129401 Files: clang/lib/

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf565444b486d: [libLTO] Set data-sections by default in libLTO. (authored by quinnp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

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

quinnp updated this revision to Diff 448016. quinnp marked 5 inline comments as done. quinnp added a comment. Addressing review comments: fixing test cases and improving test case clarity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129401/new/ h

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

MaskRay added inline comments. Comment at: llvm/test/LTO/PowerPC/data-sections-aix.ll:18-19 + +; CHECK-NOT: (csect: .data) +; CHECK: g O .data {{.*}} var + hubert.reinterpretcast wrote: > Unfortunately (without the change) t

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

hubert.reinterpretcast added a comment. With respect to the compiler changes: they look good. I checked, and the injected options being passed to the linker are added before user-specified ones (and that is good). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

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

hubert.reinterpretcast added a comment. Tests require fixing. Comment at: llvm/test/LTO/PowerPC/data-sections-aix.ll:18-19 + +; CHECK-NOT: (csect: .data) +; CHECK: g O .data {{.*}} var + Unfortunately (without the change)

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

quinnp updated this revision to Diff 447336. quinnp marked 3 inline comments as done. quinnp added a comment. Adressing review comments. Moving tests into `function-sections.c`, using `%s` instead of creating a new file with `touch`, and modifying some `CHECK` lines to simplify checks. Reposit

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

quinnp added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:579 + else if (Args.hasArg(options::OPT_fno_data_sections)) +CmdArgs.push_back("-plugin-opt=-data-sections=0"); MaskRay wrote: > Is -plugin-opt=-data-sections=0 a problem

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

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. There are some nits for tests. I'll step away and just LGTM. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:579 + else if (Args.hasArg(options::OPT_fno_data_sectio

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

quinnp updated this revision to Diff 446597. quinnp marked 3 inline comments as done. quinnp added a comment. Adressed review comments. - Modified how `llvm-lto` test-cases check the `llvm-objdump -t` output. - Renamed `gold-lto-sections.c` to `forwarding-sections-liblto.c` and modified the test

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

MaskRay added inline comments. Comment at: clang/test/Driver/forwarding-sections-liblto.c:2 +// RUN: touch %t.o +// RUN: %clang %t.o -### -flto 2>&1 | FileCheck %s +// RUN: %clang %t.o -### -flto 2>&1 -ffunction-sections -fdata-sections | \ Using a default triple

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

quinnp added a comment. In D129401#3666238 , @MaskRay wrote: > Mostly looks good, with a nit in the test and some suggestion to the summary. > > In D129401#3662857 , @quinnp wrote: > >>> If this is for the legacy

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

quinnp updated this revision to Diff 446228. quinnp marked an inline comment as done. quinnp added a comment. Addressing review comments. Changing test cases to use `llvm-objdump -t` instead of `obj2yaml`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

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

MaskRay added a comment. Mostly looks good, with a nit in the test and some suggestion to the summary. In D129401#3662857 , @quinnp wrote: >> If this is for the legacy LTO interface, please state so. `lld/*/LTO.cpp` >> sets `c.Options.DataSections = tr

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

quinnp updated this revision to Diff 446147. quinnp marked 6 inline comments as done. quinnp added a comment. Addressing review comments. Fixing the forwarding for -fno-function-sectons and removing the ObjectFormatType check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

MaskRay added a comment. I am unsure about the `llvm/lib/LTO/LTOCodeGenerator.cpp` logic. Can't your downstream project set `Config.Options.DataSections = true;` instead? Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:572 CmdArgs.push_back("-plugin-opt=-function-s

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

hubert.reinterpretcast added a comment. In D129401#3662857 , @quinnp wrote: > @hubert.reinterpretcast please correct me if I am wrong about why this change > is needed. I think your description is correct. Comment at: llvm/lib/LTO/LT

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

quinnp updated this revision to Diff 445839. quinnp added a comment. Modifying a test to fix check lines. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129401/new/ https://reviews.llvm.org/D129401 Files: clang/lib/Driver/ToolChains/CommonArgs.cp

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

quinnp added a comment. > 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` codege

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

quinnp updated this revision to Diff 445835. quinnp marked an inline comment as done. quinnp added a comment. Fixing test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129401/new/ https://reviews.llvm.org/D129401 Files: clang/lib/Driver/To

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

quinnp updated this revision to Diff 445634. quinnp added a comment. Addressing review comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129401/new/ https://reviews.llvm.org/D129401 Files: clang/lib/Driver/ToolChains/CommonArgs.cpp clang/

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. Herald added a subscriber: StephenFan. If this is for the legacy LTO interface, please state so. `lld/*/LTO.cpp` sets `c.Options.DataSections = true;` to enable data sections by de

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

quinnp updated this revision to Diff 445631. quinnp added a comment. Updating patch to forward `-data-sections=1` to `libLTO`/`gold` instead of just `-data-sections` when `-fdata-sections` is explicitly specified in `clang`. This is to be more explicit since `-data-sections=0` is now being forwa

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:579 + if (Args.hasArg(options::OPT_fno_data_sections)) +CmdArgs.push_back("-plugin-opt=-data-sections=0"); This should be combined with the previous `if` Repository:

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

quinnp updated this revision to Diff 445628. quinnp added a comment. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Updating patch with a `clang` change to properly forward `-data-sections=0` to `libLTO`/`gold` when `-fno-data-sections` is explicitly specified. R