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 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). 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
