phosek added a comment. In D115045#3175648 <https://reviews.llvm.org/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 assume it's the same for other toolchains > that override `getDefaultLinker`. > > The issue that affected AMDGPU bots is that `StringRef UseLinker = A ? > A->getValue() : ""` is now going to evaluate to an empty string unless > `-fuse-ld=` is set, and we'll take the `UseLinker.empty() || UseLinker == > "ld"` branch, where `const char *DefaultLinker = getDefaultLinker()` is going > to return `"lld"` because AMDGPU bot sets `-DCLANG_DEFAULT_LINKER=lld` in > their build. That's not a valid name, the valid name is `ld.lld`. Prior to > your patch, we would take the `!llvm::sys::path::is_absolute(UseLinker)` > branch and construct the correct linker name by appending > `CLANG_DEFAULT_LINKER` to '"ld."`. > > I'd argue that your originally patch is actually the behavior we want. > Rather, we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. > Instead AMDGPU bot should use `-DCLANG_DEFAULT_LINKER=ld.lld`. Even better > would be to introduce a second CMake variable so we can control the default > value for `-fuse-ld=` and for `--ld-path` options separately. Right now it's > not clear which of the two is controlled by `-DCLANG_DEFAULT_LINKER=` (that's > because `-DCLANG_DEFAULT_LINKER=` actually predates the `--ld-path` option). I forgot to mention, I don't think your updated logic is actually going to address the failure we've seen on the AMDGPU bot because the test failure they're seeing is not in the AMDGPU target, it's in the `x86_64-unknown-linux-gnu` target and that's still going to follow the control flow I described above. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115045/new/ https://reviews.llvm.org/D115045 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
