hubert.reinterpretcast added a comment. In D129401#3662857 <https://reviews.llvm.org/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/LTOCodeGenerator.cpp:351 + llvm::Triple::ObjectFormatType ObjectFormat = Triple.getObjectFormat(); + if (!codegen::getExplicitDataSections() && + (ObjectFormat == llvm::Triple::ObjectFormatType::ELF || ---------------- quinnp wrote: > hubert.reinterpretcast wrote: > > w2yehia wrote: > > > quinnp wrote: > > > > w2yehia wrote: > > > > > any reason we do this for ELF and XCOFF only? > > > > I don't think there is a particular reason that we do this for ELF and > > > > XCOFF only. We needed this fixed for `AIX` (`XCOFF`) and wanted to > > > > change `Linux` (`ELF`) to match the behaviour of `lld`/`gold` at the > > > > same time. I'm not sure what other file formats need for this so I did > > > > not include them. > > > > > > > > @hubert.reinterpretcast might have a better answer for this. > > > I don't know either about the other formats, was just wondering. > > > I think it's safe to do it for the file formats that we know are > > > currently different between libLTO and lld/gold. The proposed change is > > > an improvement with minimal risk. > > I agree with @w2yehia that we should change the data-sections to "on" by > > default in libLTO for the other file formats where one of lld/the gold > > plugin sets it to "on". > @hubert.reinterpretcast I think that if we want to change `data-sections` to > "on" by default for any file format which `lld` or `gold plugin` set > data-sections to "on", we would set `data-sections` to "on" for all file > formats. This is because `gold plugin` does not check the file format when it > is setting `data-sections`. You can see where `gold plugin` sets > `data-sections` here: > https://github.com/llvm/llvm-project/blob/main/llvm/tools/gold/gold-plugin.cpp#L893 > > Do you suggest that we remove the checks for file format when setting > `data-sections` in `libLTO`? ie. change the `if` statement to this: > ``` > if (!codegen::getExplicitDataSections()) > Config.Options.DataSections = true; > ``` I can't imagine setting data-sections on by default for LTO being wrong, but I am not sure that the gold-plugin supports the same range of object file formats either. I guess using `lld` as the precedent is conservatively correct? 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