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

2022-07-19 Thread Fangrui Song via Phabricator via cfe-commits
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

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

2022-07-19 Thread Hubert Tong via Phabricator via cfe-commits
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:

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

2022-07-19 Thread Quinn Pham via Phabricator via cfe-commits
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:

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

2022-07-19 Thread Quinn Pham via Phabricator via cfe-commits
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`

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

2022-07-19 Thread Quinn Pham via Phabricator via cfe-commits
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:

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

2022-07-18 Thread Quinn Pham via Phabricator via cfe-commits
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

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

2022-07-18 Thread Fangrui Song via Phabricator via cfe-commits
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

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

2022-07-18 Thread Quinn Pham via Phabricator via cfe-commits
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

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

2022-07-18 Thread Fangrui Song via Phabricator via cfe-commits
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.

2022-07-18 Thread Quinn Pham via Phabricator via cfe-commits
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.