+ Sami On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote: > > This guarding is wrong. As Documentation/kbuild/llvm.rst notes, LLVM=1 > switches the default of tools, but you can still override CC, LD, etc. > individually. > > BTW, LLVM is not 1/0 flag. If LLVM is not passed in, it is empty. Do we have the same problem with LLVM_IAS? LGTM otherwise, but wanted to check that before signing off. (Also, the rest of the patches in this series seem more related to DWARFv5 cleanups; this patch seems orthogonal while those are a visible progression). > > Non-zero return code is all treated as failure anyway. > > So, $(success,test $(LLVM) -eq 1) and $(success,test "$(LLVM)" = 1) > works equivalently in the sense that both are expanded to 'n' if LLVM > is not given. The difference is that the former internally fails due > to syntax error. > > $ test ${LLVM} -eq 1 > bash: test: -eq: unary operator expected > $ echo $? > 2 > > $ test "${LLVM}" -eq 1 > bash: test: : integer expression expected > $ echo $? > 2 > > $ test "${LLVM}" = 1 > echo $? > 1 > > $ test -n "${LLVM}" > $ echo $? > 1 > > Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx> > --- > > arch/Kconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 2bb30673d8e6..2af10ebe5ed0 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -632,7 +632,6 @@ config HAS_LTO_CLANG > def_bool y > # Clang >= 11: https://github.com/ClangBuiltLinux/linux/issues/510 > depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD > - depends on $(success,test $(LLVM) -eq 1) IIRC, we needed some other LLVM utilities like llvm-nm and llvm-ar, which are checked below. So I guess we can still support CC=clang AR=llvm-ar NM=llvm-nm, and this check is redundant. > depends on $(success,test $(LLVM_IAS) -eq 1) > depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm) > depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm) > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers