[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

tstellar added a comment. In D125624#3614298 , @ruiu wrote: > In D125624#3552770 , @tstellar > wrote: > >> In D125624#3552463 , @ruiu wrote: >> OK, as I mentioned. I

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

ruiu added a comment. In D125624#3552770 , @tstellar wrote: > In D125624#3552463 , @ruiu wrote: > >>> OK, as I mentioned. I think we need an attorney to review this change and >>> confirm that it actually accompl

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

tstellar added a subscriber: tonic. tstellar added a comment. In D125624#3552463 , @ruiu wrote: >> OK, as I mentioned. I think we need an attorney to review this change and >> confirm that it actually accomplishes this goal. > > Can you add an attorney a

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

ruiu added a comment. > OK, as I mentioned. I think we need an attorney to review this change and > confirm that it actually accomplishes this goal. Can you add an attorney as a reviewer of this change so that we can proceed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

ruiu added a comment. In D125624#3552284 , @khchen wrote: > IMO, maybe we could keep the DLLVM_BINUTILS_INCDIR option support but default > is using the Plugin.h? We can, but I wonder what is the motivation of doing it. Repository: rG LLVM Github Mo

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

khchen added a comment. IMO, maybe we could keep the DLLVM_BINUTILS_INCDIR option support but default is using the Plugin.h? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125624/new/ https://reviews.llvm.org/D125624 __

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

ruiu added a comment. > So what happens if LLVMgold.so uses one of the new structs or constants and > then is built and run on system where binutils is old enough to not have > these new structs and constants? The same thing can happen with GCC and binutils. Imagine that you install a newer

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

tstellar added a comment. In D125624#3552238 , @ruiu wrote: > The motivation of doing this is to be able to build LLVMgold.so without > binutils' source files and make it clear that LLVMgold.so does not include > any GPL code. OK, as I mentioned. I th

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

ruiu added a comment. The motivation of doing this is to be able to build LLVMgold.so without binutils' source files and make it clear that LLVMgold.so does not include any GPL code. The header file defines the public interface between linker plugins and compilers (and other tools such as `ar`

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

tstellar requested changes to this revision. tstellar added a comment. This revision now requires changes to proceed. What is the motivation for this change? I really don't like bundling external headers like this, because it can lead to subtle hard to catch bugs. It also makes it harder to di

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

ruiu added a comment. I'm not sure if my credential is still valid. Do you mind if I ask you to submit this for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125624/new/ https://reviews.llvm.org/D125624 ___

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

ruiu added a comment. Does it? What is your test command? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125624/new/ https://reviews.llvm.org/D125624 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

MaskRay added a comment. It's better to remove LLVM_BINUTILS_INCDIR for test directories in another patch. This change may expose some failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125624/new/ https://reviews.llvm.org/D125624 __

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

ruiu added inline comments. Herald added a subscriber: Enna1. Comment at: llvm/tools/gold/gold-plugin.cpp:44-52 // FIXME: remove this declaration when we stop maintaining Ubuntu Quantal and // Precise and Debian Wheezy (binutils 2.23 is required) #define LDPO_PIE 3 #define

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

pcc added inline comments. Comment at: llvm/tools/gold/gold-plugin.cpp:44-52 // FIXME: remove this declaration when we stop maintaining Ubuntu Quantal and // Precise and Debian Wheezy (binutils 2.23 is required) #define LDPO_PIE 3 #define LDPT_GET_SYMBOLS_V3 28 // FIXME:

[PATCH] D125624: [gold] Remove an external dependency to GNU binutils' header file

MaskRay added a comment. I haven't checked but for `ninja check-llvm-tools-gold`, we still hope that the tests are disabled if the system does not provide gold. There may need a toggle for the tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1