From: Nick Desaulniers <ndesaulniers@google.com>
To: Masahiro Yamada <masahiroy@kernel.org>,
Sami Tolvanen <samitolvanen@google.com>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
clang-built-linux <clang-built-linux@googlegroups.com>,
Nathan Chancellor <nathan@kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG
Date: Wed, 3 Mar 2021 12:47:27 -0800 [thread overview]
Message-ID: <CAKwvOdkhZGv_q9vgDdYY44OrbzmMD_E+GL3SyOk-jQ0kdXtMzg@mail.gmail.com> (raw)
In-Reply-To: <20210303183333.46543-1-masahiroy@kernel.org>
+ Sami
On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada <masahiroy@kernel.org> 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@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.
> 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
next prev parent reply other threads:[~2021-03-04 0:08 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-03 18:33 [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Masahiro Yamada
2021-03-03 18:33 ` [PATCH 2/4] kbuild: collect minimum tool versions into scripts/tool-version.sh Masahiro Yamada
2021-03-04 0:11 ` Nathan Chancellor
2021-03-05 6:40 ` Masahiro Yamada
2021-03-12 17:21 ` Masahiro Yamada
2021-03-04 22:10 ` Nicolas Pitre
2021-03-05 6:41 ` Masahiro Yamada
2021-03-03 18:33 ` [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig Masahiro Yamada
2021-03-04 0:28 ` Nathan Chancellor
2021-03-05 1:25 ` Nick Desaulniers
2021-03-05 17:48 ` Masahiro Yamada
2021-03-07 4:42 ` Nathan Chancellor
2021-03-03 18:33 ` [PATCH 4/4] kbuild: dwarf: use AS_VERSION instead of test_dwarf5_support.sh Masahiro Yamada
2021-03-03 20:44 ` Nick Desaulniers
2021-03-12 17:18 ` Masahiro Yamada
2021-03-04 0:30 ` Nathan Chancellor
2021-03-03 20:47 ` Nick Desaulniers [this message]
2021-03-05 6:06 ` [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Masahiro Yamada
2021-03-08 19:11 ` Sami Tolvanen
2021-03-03 23:37 ` Nathan Chancellor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKwvOdkhZGv_q9vgDdYY44OrbzmMD_E+GL3SyOk-jQ0kdXtMzg@mail.gmail.com \
--to=ndesaulniers@google.com \
--cc=clang-built-linux@googlegroups.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=masahiroy@kernel.org \
--cc=nathan@kernel.org \
--cc=samitolvanen@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.