Thread (20 messages) 20 messages, 5 authors, 2021-03-12

Re: [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG

From: Masahiro Yamada <masahiroy@kernel.org>
Date: 2021-03-05 06:07:05
Also in: lkml

On Thu, Mar 4, 2021 at 5:47 AM 'Nick Desaulniers' via Clang Built
Linux [off-list ref] wrote:
+ Sami

On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada [off-list ref] wrote:
quoted
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.
3/4 will replace this LLVM_IAS check with AS_IS_LLVM.

We do not need to add a noise change.



(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).
Kind of orthogonal, but I am touching the same code hunk,
which would cause a merge conflict.

quoted
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@kernel.org>
---

 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.
Yes, I think so.

quoted
        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

--
You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdkhZGv_q9vgDdYY44OrbzmMD_E%2BGL3SyOk-jQ0kdXtMzg%40mail.gmail.com.


-- 
Best Regards
Masahiro Yamada
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help