[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-07-02 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added a comment. > Personally I am opposed to this change. The compiler driver (gcc,clang) has a > set of arch/OS dependent defaults. It seems weird/redundant to add another > sets of defaults on the linker side. The NetBSD toolchain relies on internal knowledge in a linker that

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-07-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. > krytarowski: The discussion is ongoing since 2017 and we have an impasse. > >> peter.smith: I think this might be better raised on lllvm-dev as I suspect >> that we need to give this wider visibility. I'm not totally opposed

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-07-01 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added a comment. Herald added a subscriber: MaskRay. @ruiu ping? LLVM 9 is branching soon and we would like to unearth from local patches. From the internal discussions it was decided that we want to maintain our current behavior that differs to Linux. CHANGES SINCE LAST ACTION

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added a comment. > chandlerc added a comment. > > There was a long discussion between two NetBSD maintainers about this (both > already in the reviewers list of this patch). I'm not sure if there is an > existing thread that would be better to follow up on as opposed to starting a

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added a comment. In D56650#1377546 , @ruiu wrote: > If you still need to patch GNU ld, it doesn't seems like this patch makes > things easier for you. (But even if this would make it easier for you, this > patch's approach is not okay by

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added a comment. If we pass flags from clang, we sacrifice: - lld usable as a standalone executable - gcc capable to use lld as a functional linker - clang driver not capable to call unpatched gnu ld/gold as we grow local flags to workaroud the customization (such as -z nognustack)

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added a comment. If you still need to patch GNU ld, it doesn't seems like this patch makes things easier for you. (But even if this would make it easier for you, this patch's approach is not okay by design though.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56650/new/

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Michał Górny via Phabricator via cfe-commits
mgorny added a comment. In D56650#1377494 , @ruiu wrote: > But that's only adding missing `-L` and perhaps a few more, no? That doesn't > seem too hard to do in gcc. Probably not, provided nobody will block it. I'm not the one opposed to it. > I don't

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added a comment. But that's only adding missing `-L` and perhaps a few more, no? That doesn't seem too hard to do in gcc. I don't want to repeat what compiler drivers do in the linker. Also, even with this patch, you need to make a change to gcc to pass `--target` parameter to the linker,

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Michał Górny via Phabricator via cfe-commits
mgorny added a comment. I think the main concern is that if we link `ld -> ld.lld`, gcc will no longer work out of the box. So we would have to repeat all the logic in gcc and in any other compiler that calls `ld` directly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56650/new/

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added a comment. Kamil, I understand how adding `--target` option to the linker would make target-specific customization easier, but that's as I said not acceptable by design in lld. The code we have for FreeBSD just sets one bit in the output file, and that's fundamentally different

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-30 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added a comment. @rui we need some resolution here. We have stronger feelings from the community to customize the linker directly based on detected triple. At least other !GNU platforms will benefit from it too (at least FreeBSD, OpenBSD and other BSD derivations like mentioned

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-14 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu accepted this revision. ruiu added a comment. This revision is now accepted and ready to land. Let me make it clear again that I'm *not* okay with this approach. Please explicitly pass command line arguments from the compiler driver to the linker. CHANGES SINCE LAST ACTION

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-14 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment. I think we need to be very careful here. If I've understood this correctly then if this functionality is used for anything critical then a manually supplied target will be needed when doing cross-linking. For example my host LLD is x86_64, is just called ld.lld and

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-14 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 181554. mgorny marked an inline comment as done. mgorny added a comment. Fixed leaving triple unset on invalid `--target`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56650/new/ https://reviews.llvm.org/D56650 Files: ELF/Config.h

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-14 Thread Michał Górny via Phabricator via cfe-commits
mgorny marked 2 inline comments as done. mgorny added inline comments. Comment at: ELF/Driver.cpp:757 + if (!TargetOpt.empty()) { +// TODO: do we want to verify it and fail on unsupported? +Config->TargetTriple = llvm::Triple(TargetOpt); arichardson

[PATCH] D56650: [lld] [ELF] Support customizing behavior on target triple

2019-01-14 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 181552. mgorny retitled this revision from "[lld] [ELF] Support inferring target triple from filename" to "[lld] [ELF] Support customizing behavior on target triple". mgorny edited the summary of this revision. CHANGES SINCE LAST ACTION