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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
