[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-07 Thread Simon Moll via Phabricator via cfe-commits
simoll added a comment. In D115045#3175648 , @phosek wrote: > I don't think the updated logic is correct. For example, in the case of > Fuchsia we don't want to take `CLANG_DEFAULT_LINKER` into account, that's why > we override `getDefaultLinker`. I ass

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-07 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. In D115045#3175760 , @mstorsjo wrote: > In D115045#3175648 , @phosek wrote: > >> Rather, we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. >> Instead AMDGPU bot should u

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-07 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D115045#3175648 , @phosek wrote: > Rather, we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. > Instead AMDGPU bot should use `-DCLANG_DEFAULT_LINKER=ld.lld`. I think this would be a kinda disruptive change

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-07 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. In D115045#3175648 , @phosek wrote: > I don't think the updated logic is correct. For example, in the case of > Fuchsia we don't want to take `CLANG_DEFAULT_LINKER` into account, that's why > we override `getDefaultLinker`. I ass

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-07 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment. I don't think the updated logic is correct. For example, in the case of Fuchsia we don't want to take `CLANG_DEFAULT_LINKER` into account, that's why we override `getDefaultLinker`. I assume it's the same for other toolchains that override `getDefaultLinker`. The issue

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-07 Thread Simon Moll via Phabricator via cfe-commits
simoll updated this revision to Diff 392297. simoll added a comment. Formatting / Cleanup Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115045/new/ https://reviews.llvm.org/D115045 Files: clang/include/clang/Driver/ToolChain.h clang/lib/Driver

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-06 Thread Simon Moll via Phabricator via cfe-commits
simoll updated this revision to Diff 392067. simoll added a comment. Herald added subscribers: abrachet, kerbowa, ormris, atanasyan, jrtc27, aheejin, jgravelle-google, sbc100, nhaehnle, jvesely, sdardis, dschuff. Use the `-DCLANG_DEFAULT_LINKER` linker in all toolchains but VE. Since toolchains

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-06 Thread Ron Lieberman via Phabricator via cfe-commits
ronlieb added a comment. fyi: if i remove the now 'dead' use of the CMAKE variable from my cmake , then i am able to build. so i see you reverted, thx. i guess some coordination amongst buildbot maintainers who might use this option checking zorg: the last two are me: not sure who owns the 1

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-06 Thread Simon Moll via Phabricator via cfe-commits
simoll reopened this revision. simoll added a comment. This revision is now accepted and ready to land. I've reverted the patch for now, this may show up for other toolchains, too. This patch has pushed down the responsibility for handling `-DCLANG_DEFAULT_LINKER` to the toolchains. However, I d

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-06 Thread Ron Lieberman via Phabricator via cfe-commits
ronlieb added a comment. Hi Simon, i am seeing a failure in our amdgpu buildbot after this patch . https://lab.llvm.org/staging/#/builders/200/builds/1407 we do depend on the cmake flag you removed. we specify this -DCLANG_DEFAULT_LINKER=lld FAILED: openmp/libomptarget/libomptarget.rtl.x86_64.s

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-06 Thread Simon Moll via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG34a43f2115af: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains (authored by simoll). Changed prior to commit: https://reviews.llvm.org/D115045?vs=391998&id=392022#toc Repository: rG L

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-06 Thread Simon Moll via Phabricator via cfe-commits
simoll updated this revision to Diff 391998. simoll marked an inline comment as done. simoll added a comment. Simplified empty-string check as suggested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115045/new/ https://reviews.llvm.org/D115045 Fil

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. LGTM. Comment at: clang/lib/Driver/ToolChain.cpp:545 +const char *ToolChain::getDefaultLinker() const { + if (StringRef(CLANG_DEFAULT_LINKER).empty()) +return "ld"; `CLANG_DEFAUALT_LINKER[0] == '\0'`

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-03 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision. phosek added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115045/new/ https://reviews.llvm.org/D115045 ___

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-03 Thread Simon Moll via Phabricator via cfe-commits
simoll created this revision. simoll added reviewers: kaz7, MaskRay, phosek. simoll requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Before, the CLANG_DEFAULT_LINKER cmake option was a global override for the linker that shall be used on all