* [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG @ 2021-03-03 18:33 Masahiro Yamada 2021-03-03 18:33 ` [PATCH 2/4] kbuild: collect minimum tool versions into scripts/tool-version.sh Masahiro Yamada ` (4 more replies) 0 siblings, 5 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-03 18:33 UTC (permalink / raw) To: linux-kbuild Cc: clang-built-linux, Nick Desaulniers, Masahiro Yamada, Nathan Chancellor, linux-kernel 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. 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) 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/4] kbuild: collect minimum tool versions into scripts/tool-version.sh 2021-03-03 18:33 [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Masahiro Yamada @ 2021-03-03 18:33 ` Masahiro Yamada 2021-03-04 0:11 ` Nathan Chancellor 2021-03-04 22:10 ` Nicolas Pitre 2021-03-03 18:33 ` [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig Masahiro Yamada ` (3 subsequent siblings) 4 siblings, 2 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-03 18:33 UTC (permalink / raw) To: linux-kbuild Cc: clang-built-linux, Nick Desaulniers, Masahiro Yamada, Nathan Chancellor, linux-kernel The kernel build uses various tools, many of which are provided by the same software suite, for example, LLVM and Binutils. When we raise the minimal version of Clang/LLVM, we need to update clang_min_version in scripts/cc-version.sh and also lld_min_version in scripts/ld-version.sh. In fact, Kbuild can handle CC=clang and LD=ld.lld independently, and we could manage their minimal version separately, but it does not make much sense. Make scripts/tool-version.sh a central place of minimum tool versions so that we do not need to touch multiple files. This script prints the minimal version of the given tool. $ scripts/tool-version.sh gcc 4.9.0 $ scripts/tool-version.sh llvm 10.0.1 $ scripts/tool-version.sh binutils 2.23.0 $ scripts/tool-version.sh foo foo: unknown tool Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/cc-version.sh | 20 +++++--------------- scripts/ld-version.sh | 11 ++++------- scripts/tool-version.sh | 27 +++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 22 deletions(-) create mode 100755 scripts/tool-version.sh diff --git a/scripts/cc-version.sh b/scripts/cc-version.sh index 3f2ee885b116..4772f1ef9cac 100755 --- a/scripts/cc-version.sh +++ b/scripts/cc-version.sh @@ -6,18 +6,6 @@ set -e -# When you raise the minimum compiler version, please update -# Documentation/process/changes.rst as well. -gcc_min_version=4.9.0 -clang_min_version=10.0.1 -icc_min_version=16.0.3 # temporary - -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 -# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk -if [ "$SRCARCH" = arm64 ]; then - gcc_min_version=5.1.0 -fi - # Print the compiler name and some version components. get_compiler_info() { @@ -48,18 +36,20 @@ set -- $(get_compiler_info "$@") name=$1 +tool_version=$(dirname $0)/tool-version.sh + case "$name" in GCC) version=$2.$3.$4 - min_version=$gcc_min_version + min_version=$($tool_version gcc) ;; Clang) version=$2.$3.$4 - min_version=$clang_min_version + min_version=$($tool_version llvm) ;; ICC) version=$(($2 / 100)).$(($2 % 100)).$3 - min_version=$icc_min_version + min_version=$($tool_version icc) ;; *) echo "$orig_args: unknown compiler" >&2 diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh index a463273509b5..e824f7675693 100755 --- a/scripts/ld-version.sh +++ b/scripts/ld-version.sh @@ -6,11 +6,6 @@ set -e -# When you raise the minimum linker version, please update -# Documentation/process/changes.rst as well. -bfd_min_version=2.23.0 -lld_min_version=10.0.1 - # Convert the version string x.y.z to a canonical 5 or 6-digit form. get_canonical_version() { @@ -35,10 +30,12 @@ set -- $("$@" --version) IFS=' ' set -- $1 +tool_version=$(dirname $0)/tool-version.sh + if [ "$1" = GNU -a "$2" = ld ]; then shift $(($# - 1)) version=$1 - min_version=$bfd_min_version + min_version=$($tool_version binutils) name=BFD disp_name="GNU ld" elif [ "$1" = GNU -a "$2" = gold ]; then @@ -46,7 +43,7 @@ elif [ "$1" = GNU -a "$2" = gold ]; then exit 1 elif [ "$1" = LLD ]; then version=$2 - min_version=$lld_min_version + min_version=$($tool_version llvm) name=LLD disp_name=LLD else diff --git a/scripts/tool-version.sh b/scripts/tool-version.sh new file mode 100755 index 000000000000..b4aa27e2c3d3 --- /dev/null +++ b/scripts/tool-version.sh @@ -0,0 +1,27 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0-only +# +# Print the minimum supported version of the given tool. + +set -e + +# When you raise the minimum version, please update +# Documentation/process/changes.rst as well. +gcc_min_version=4.9.0 +llvm_min_version=10.0.1 +icc_min_version=16.0.3 # temporary +binutils_min_version=2.23.0 + +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 +# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk +if [ "$SRCARCH" = arm64 ]; then + gcc_min_version=5.1.0 +fi + +eval min_version="\$${1}_min_version" +if [ -z "$min_version" ]; then + echo "$1: unknown tool" >&2 + exit 1 +fi + +echo "$min_version" -- 2.27.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] kbuild: collect minimum tool versions into scripts/tool-version.sh 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 1 sibling, 2 replies; 20+ messages in thread From: Nathan Chancellor @ 2021-03-04 0:11 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, clang-built-linux, Nick Desaulniers, linux-kernel On Thu, Mar 04, 2021 at 03:33:31AM +0900, Masahiro Yamada wrote: > The kernel build uses various tools, many of which are provided by the > same software suite, for example, LLVM and Binutils. > > When we raise the minimal version of Clang/LLVM, we need to update > clang_min_version in scripts/cc-version.sh and also lld_min_version in > scripts/ld-version.sh. > > In fact, Kbuild can handle CC=clang and LD=ld.lld independently, and we > could manage their minimal version separately, but it does not make > much sense. > > Make scripts/tool-version.sh a central place of minimum tool versions > so that we do not need to touch multiple files. > > This script prints the minimal version of the given tool. > > $ scripts/tool-version.sh gcc > 4.9.0 > $ scripts/tool-version.sh llvm > 10.0.1 > $ scripts/tool-version.sh binutils > 2.23.0 > $ scripts/tool-version.sh foo > foo: unknown tool > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Two comments below. > --- > > scripts/cc-version.sh | 20 +++++--------------- > scripts/ld-version.sh | 11 ++++------- > scripts/tool-version.sh | 27 +++++++++++++++++++++++++++ > 3 files changed, 36 insertions(+), 22 deletions(-) > create mode 100755 scripts/tool-version.sh > > diff --git a/scripts/cc-version.sh b/scripts/cc-version.sh > index 3f2ee885b116..4772f1ef9cac 100755 > --- a/scripts/cc-version.sh > +++ b/scripts/cc-version.sh > @@ -6,18 +6,6 @@ > > set -e > > -# When you raise the minimum compiler version, please update > -# Documentation/process/changes.rst as well. > -gcc_min_version=4.9.0 > -clang_min_version=10.0.1 > -icc_min_version=16.0.3 # temporary > - > -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > -# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk > -if [ "$SRCARCH" = arm64 ]; then > - gcc_min_version=5.1.0 > -fi > - > # Print the compiler name and some version components. > get_compiler_info() > { > @@ -48,18 +36,20 @@ set -- $(get_compiler_info "$@") > > name=$1 > > +tool_version=$(dirname $0)/tool-version.sh I realize these scripts are currently called by their full path but is it worth making this '$(dirname "$(readlink -f "$0")")' here and in ld-version.sh just in case that does not happen? > case "$name" in > GCC) > version=$2.$3.$4 > - min_version=$gcc_min_version > + min_version=$($tool_version gcc) > ;; > Clang) > version=$2.$3.$4 > - min_version=$clang_min_version > + min_version=$($tool_version llvm) > ;; > ICC) > version=$(($2 / 100)).$(($2 % 100)).$3 > - min_version=$icc_min_version > + min_version=$($tool_version icc) > ;; > *) > echo "$orig_args: unknown compiler" >&2 > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > index a463273509b5..e824f7675693 100755 > --- a/scripts/ld-version.sh > +++ b/scripts/ld-version.sh > @@ -6,11 +6,6 @@ > > set -e > > -# When you raise the minimum linker version, please update > -# Documentation/process/changes.rst as well. > -bfd_min_version=2.23.0 > -lld_min_version=10.0.1 > - > # Convert the version string x.y.z to a canonical 5 or 6-digit form. > get_canonical_version() > { > @@ -35,10 +30,12 @@ set -- $("$@" --version) > IFS=' ' > set -- $1 > > +tool_version=$(dirname $0)/tool-version.sh > + > if [ "$1" = GNU -a "$2" = ld ]; then > shift $(($# - 1)) > version=$1 > - min_version=$bfd_min_version > + min_version=$($tool_version binutils) > name=BFD > disp_name="GNU ld" > elif [ "$1" = GNU -a "$2" = gold ]; then > @@ -46,7 +43,7 @@ elif [ "$1" = GNU -a "$2" = gold ]; then > exit 1 > elif [ "$1" = LLD ]; then > version=$2 > - min_version=$lld_min_version > + min_version=$($tool_version llvm) > name=LLD > disp_name=LLD > else > diff --git a/scripts/tool-version.sh b/scripts/tool-version.sh > new file mode 100755 > index 000000000000..b4aa27e2c3d3 > --- /dev/null > +++ b/scripts/tool-version.sh > @@ -0,0 +1,27 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Print the minimum supported version of the given tool. > + > +set -e > + > +# When you raise the minimum version, please update > +# Documentation/process/changes.rst as well. > +gcc_min_version=4.9.0 > +llvm_min_version=10.0.1 > +icc_min_version=16.0.3 # temporary > +binutils_min_version=2.23.0 > + > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > +# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk > +if [ "$SRCARCH" = arm64 ]; then > + gcc_min_version=5.1.0 > +fi > + > +eval min_version="\$${1}_min_version" > +if [ -z "$min_version" ]; then > + echo "$1: unknown tool" >&2 > + exit 1 > +fi > + > +echo "$min_version" > -- > 2.27.0 > Would scripts/tool-version.sh be easier to read and interpret using a case statement? #!/bin/sh # SPDX-License-Identifier: GPL-2.0-only # # Print the minimum supported version of the given tool. # When you raise the minimum version, please update # Documentation/process/changes.rst as well. case "$1" in binutils) echo "2.23.0" ;; gcc) # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 # https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk if [ "$SRCARCH" = arm64 ]; then echo "5.1.0" else echo "4.9.0" fi ;; icc) # temporary echo "16.0.3" ;; llvm) echo "10.0.1" ;; *) echo "$1: unknown tool" >&2 exit 1 ;; esac ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] kbuild: collect minimum tool versions into scripts/tool-version.sh 2021-03-04 0:11 ` Nathan Chancellor @ 2021-03-05 6:40 ` Masahiro Yamada 2021-03-12 17:21 ` Masahiro Yamada 1 sibling, 0 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-05 6:40 UTC (permalink / raw) To: Nathan Chancellor Cc: Linux Kbuild mailing list, clang-built-linux, Nick Desaulniers, Linux Kernel Mailing List On Thu, Mar 4, 2021 at 9:11 AM Nathan Chancellor <nathan@kernel.org> wrote: > > On Thu, Mar 04, 2021 at 03:33:31AM +0900, Masahiro Yamada wrote: > > The kernel build uses various tools, many of which are provided by the > > same software suite, for example, LLVM and Binutils. > > > > When we raise the minimal version of Clang/LLVM, we need to update > > clang_min_version in scripts/cc-version.sh and also lld_min_version in > > scripts/ld-version.sh. > > > > In fact, Kbuild can handle CC=clang and LD=ld.lld independently, and we > > could manage their minimal version separately, but it does not make > > much sense. > > > > Make scripts/tool-version.sh a central place of minimum tool versions > > so that we do not need to touch multiple files. > > > > This script prints the minimal version of the given tool. > > > > $ scripts/tool-version.sh gcc > > 4.9.0 > > $ scripts/tool-version.sh llvm > > 10.0.1 > > $ scripts/tool-version.sh binutils > > 2.23.0 > > $ scripts/tool-version.sh foo > > foo: unknown tool > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > Two comments below. > > > --- > > > > scripts/cc-version.sh | 20 +++++--------------- > > scripts/ld-version.sh | 11 ++++------- > > scripts/tool-version.sh | 27 +++++++++++++++++++++++++++ > > 3 files changed, 36 insertions(+), 22 deletions(-) > > create mode 100755 scripts/tool-version.sh > > > > diff --git a/scripts/cc-version.sh b/scripts/cc-version.sh > > index 3f2ee885b116..4772f1ef9cac 100755 > > --- a/scripts/cc-version.sh > > +++ b/scripts/cc-version.sh > > @@ -6,18 +6,6 @@ > > > > set -e > > > > -# When you raise the minimum compiler version, please update > > -# Documentation/process/changes.rst as well. > > -gcc_min_version=4.9.0 > > -clang_min_version=10.0.1 > > -icc_min_version=16.0.3 # temporary > > - > > -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > > -# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk > > -if [ "$SRCARCH" = arm64 ]; then > > - gcc_min_version=5.1.0 > > -fi > > - > > # Print the compiler name and some version components. > > get_compiler_info() > > { > > @@ -48,18 +36,20 @@ set -- $(get_compiler_info "$@") > > > > name=$1 > > > > +tool_version=$(dirname $0)/tool-version.sh > > I realize these scripts are currently called by their full path but is > it worth making this '$(dirname "$(readlink -f "$0")")' here and in > ld-version.sh just in case that does not happen? Not sure. A relative path still works. So, I did not do what I did not need to do. > > case "$name" in > > GCC) > > version=$2.$3.$4 > > - min_version=$gcc_min_version > > + min_version=$($tool_version gcc) > > ;; > > Clang) > > version=$2.$3.$4 > > - min_version=$clang_min_version > > + min_version=$($tool_version llvm) > > ;; > > ICC) > > version=$(($2 / 100)).$(($2 % 100)).$3 > > - min_version=$icc_min_version > > + min_version=$($tool_version icc) > > ;; > > *) > > echo "$orig_args: unknown compiler" >&2 > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > > index a463273509b5..e824f7675693 100755 > > --- a/scripts/ld-version.sh > > +++ b/scripts/ld-version.sh > > @@ -6,11 +6,6 @@ > > > > set -e > > > > -# When you raise the minimum linker version, please update > > -# Documentation/process/changes.rst as well. > > -bfd_min_version=2.23.0 > > -lld_min_version=10.0.1 > > - > > # Convert the version string x.y.z to a canonical 5 or 6-digit form. > > get_canonical_version() > > { > > @@ -35,10 +30,12 @@ set -- $("$@" --version) > > IFS=' ' > > set -- $1 > > > > +tool_version=$(dirname $0)/tool-version.sh > > + > > if [ "$1" = GNU -a "$2" = ld ]; then > > shift $(($# - 1)) > > version=$1 > > - min_version=$bfd_min_version > > + min_version=$($tool_version binutils) > > name=BFD > > disp_name="GNU ld" > > elif [ "$1" = GNU -a "$2" = gold ]; then > > @@ -46,7 +43,7 @@ elif [ "$1" = GNU -a "$2" = gold ]; then > > exit 1 > > elif [ "$1" = LLD ]; then > > version=$2 > > - min_version=$lld_min_version > > + min_version=$($tool_version llvm) > > name=LLD > > disp_name=LLD > > else > > diff --git a/scripts/tool-version.sh b/scripts/tool-version.sh > > new file mode 100755 > > index 000000000000..b4aa27e2c3d3 > > --- /dev/null > > +++ b/scripts/tool-version.sh > > @@ -0,0 +1,27 @@ > > +#!/bin/sh > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# Print the minimum supported version of the given tool. > > + > > +set -e > > + > > +# When you raise the minimum version, please update > > +# Documentation/process/changes.rst as well. > > +gcc_min_version=4.9.0 > > +llvm_min_version=10.0.1 > > +icc_min_version=16.0.3 # temporary > > +binutils_min_version=2.23.0 > > + > > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > > +# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk > > +if [ "$SRCARCH" = arm64 ]; then > > + gcc_min_version=5.1.0 > > +fi > > + > > +eval min_version="\$${1}_min_version" > > +if [ -z "$min_version" ]; then > > + echo "$1: unknown tool" >&2 > > + exit 1 > > +fi > > + > > +echo "$min_version" > > -- > > 2.27.0 > > > > Would scripts/tool-version.sh be easier to read and interpret using a > case statement? Actually, I wrote code that way. But, I liked shorter code, and wanted to collect magic numbers to the top of this file. Only the tricky part is the 'eval' statement, but I hope it would not be too difficult to understand. > #!/bin/sh > # SPDX-License-Identifier: GPL-2.0-only > # > # Print the minimum supported version of the given tool. > # When you raise the minimum version, please update > # Documentation/process/changes.rst as well. > > case "$1" in > binutils) > echo "2.23.0" > ;; > gcc) > # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > # https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk > if [ "$SRCARCH" = arm64 ]; then > echo "5.1.0" > else > echo "4.9.0" > fi > ;; > icc) > # temporary > echo "16.0.3" > ;; > llvm) > echo "10.0.1" > ;; > *) > echo "$1: unknown tool" >&2 > exit 1 > ;; > esac > > -- > 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/20210304001141.7lejurony2poqkid%40archlinux-ax161. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] kbuild: collect minimum tool versions into scripts/tool-version.sh 2021-03-04 0:11 ` Nathan Chancellor 2021-03-05 6:40 ` Masahiro Yamada @ 2021-03-12 17:21 ` Masahiro Yamada 1 sibling, 0 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-12 17:21 UTC (permalink / raw) To: Nathan Chancellor Cc: Linux Kbuild mailing list, clang-built-linux, Nick Desaulniers, Linux Kernel Mailing List On Thu, Mar 4, 2021 at 9:11 AM Nathan Chancellor <nathan@kernel.org> wrote: > > On Thu, Mar 04, 2021 at 03:33:31AM +0900, Masahiro Yamada wrote: > > The kernel build uses various tools, many of which are provided by the > > same software suite, for example, LLVM and Binutils. > > > > When we raise the minimal version of Clang/LLVM, we need to update > > clang_min_version in scripts/cc-version.sh and also lld_min_version in > > scripts/ld-version.sh. > > > > In fact, Kbuild can handle CC=clang and LD=ld.lld independently, and we > > could manage their minimal version separately, but it does not make > > much sense. > > > > Make scripts/tool-version.sh a central place of minimum tool versions > > so that we do not need to touch multiple files. > > > > This script prints the minimal version of the given tool. > > > > $ scripts/tool-version.sh gcc > > 4.9.0 > > $ scripts/tool-version.sh llvm > > 10.0.1 > > $ scripts/tool-version.sh binutils > > 2.23.0 > > $ scripts/tool-version.sh foo > > foo: unknown tool > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > Two comments below. > > > --- > > > > scripts/cc-version.sh | 20 +++++--------------- > > scripts/ld-version.sh | 11 ++++------- > > scripts/tool-version.sh | 27 +++++++++++++++++++++++++++ > > 3 files changed, 36 insertions(+), 22 deletions(-) > > create mode 100755 scripts/tool-version.sh > > > > diff --git a/scripts/cc-version.sh b/scripts/cc-version.sh > > index 3f2ee885b116..4772f1ef9cac 100755 > > --- a/scripts/cc-version.sh > > +++ b/scripts/cc-version.sh > > @@ -6,18 +6,6 @@ > > > > set -e > > > > -# When you raise the minimum compiler version, please update > > -# Documentation/process/changes.rst as well. > > -gcc_min_version=4.9.0 > > -clang_min_version=10.0.1 > > -icc_min_version=16.0.3 # temporary > > - > > -# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > > -# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk > > -if [ "$SRCARCH" = arm64 ]; then > > - gcc_min_version=5.1.0 > > -fi > > - > > # Print the compiler name and some version components. > > get_compiler_info() > > { > > @@ -48,18 +36,20 @@ set -- $(get_compiler_info "$@") > > > > name=$1 > > > > +tool_version=$(dirname $0)/tool-version.sh > > I realize these scripts are currently called by their full path but is > it worth making this '$(dirname "$(readlink -f "$0")")' here and in > ld-version.sh just in case that does not happen? > > > case "$name" in > > GCC) > > version=$2.$3.$4 > > - min_version=$gcc_min_version > > + min_version=$($tool_version gcc) > > ;; > > Clang) > > version=$2.$3.$4 > > - min_version=$clang_min_version > > + min_version=$($tool_version llvm) > > ;; > > ICC) > > version=$(($2 / 100)).$(($2 % 100)).$3 > > - min_version=$icc_min_version > > + min_version=$($tool_version icc) > > ;; > > *) > > echo "$orig_args: unknown compiler" >&2 > > diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh > > index a463273509b5..e824f7675693 100755 > > --- a/scripts/ld-version.sh > > +++ b/scripts/ld-version.sh > > @@ -6,11 +6,6 @@ > > > > set -e > > > > -# When you raise the minimum linker version, please update > > -# Documentation/process/changes.rst as well. > > -bfd_min_version=2.23.0 > > -lld_min_version=10.0.1 > > - > > # Convert the version string x.y.z to a canonical 5 or 6-digit form. > > get_canonical_version() > > { > > @@ -35,10 +30,12 @@ set -- $("$@" --version) > > IFS=' ' > > set -- $1 > > > > +tool_version=$(dirname $0)/tool-version.sh > > + > > if [ "$1" = GNU -a "$2" = ld ]; then > > shift $(($# - 1)) > > version=$1 > > - min_version=$bfd_min_version > > + min_version=$($tool_version binutils) > > name=BFD > > disp_name="GNU ld" > > elif [ "$1" = GNU -a "$2" = gold ]; then > > @@ -46,7 +43,7 @@ elif [ "$1" = GNU -a "$2" = gold ]; then > > exit 1 > > elif [ "$1" = LLD ]; then > > version=$2 > > - min_version=$lld_min_version > > + min_version=$($tool_version llvm) > > name=LLD > > disp_name=LLD > > else > > diff --git a/scripts/tool-version.sh b/scripts/tool-version.sh > > new file mode 100755 > > index 000000000000..b4aa27e2c3d3 > > --- /dev/null > > +++ b/scripts/tool-version.sh > > @@ -0,0 +1,27 @@ > > +#!/bin/sh > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# Print the minimum supported version of the given tool. > > + > > +set -e > > + > > +# When you raise the minimum version, please update > > +# Documentation/process/changes.rst as well. > > +gcc_min_version=4.9.0 > > +llvm_min_version=10.0.1 > > +icc_min_version=16.0.3 # temporary > > +binutils_min_version=2.23.0 > > + > > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > > +# https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk > > +if [ "$SRCARCH" = arm64 ]; then > > + gcc_min_version=5.1.0 > > +fi > > + > > +eval min_version="\$${1}_min_version" > > +if [ -z "$min_version" ]; then > > + echo "$1: unknown tool" >&2 > > + exit 1 > > +fi > > + > > +echo "$min_version" > > -- > > 2.27.0 > > > > Would scripts/tool-version.sh be easier to read and interpret using a > case statement? > > #!/bin/sh > # SPDX-License-Identifier: GPL-2.0-only > # > # Print the minimum supported version of the given tool. > # When you raise the minimum version, please update > # Documentation/process/changes.rst as well. > > case "$1" in > binutils) > echo "2.23.0" > ;; > gcc) > # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63293 > # https://lore.kernel.org/r/20210107111841.GN1551@shell.armlinux.org.uk > if [ "$SRCARCH" = arm64 ]; then > echo "5.1.0" > else > echo "4.9.0" > fi > ;; > icc) > # temporary > echo "16.0.3" > ;; > llvm) > echo "10.0.1" > ;; > *) > echo "$1: unknown tool" >&2 > exit 1 > ;; > esac > After some more thoughts, I think your suggestion is better. My patch is fragile in case the user sets an environment variable "foo_min_version". -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] kbuild: collect minimum tool versions into scripts/tool-version.sh 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-04 22:10 ` Nicolas Pitre 2021-03-05 6:41 ` Masahiro Yamada 1 sibling, 1 reply; 20+ messages in thread From: Nicolas Pitre @ 2021-03-04 22:10 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, clang-built-linux, Nick Desaulniers, Nathan Chancellor, linux-kernel On Thu, 4 Mar 2021, Masahiro Yamada wrote: > The kernel build uses various tools, many of which are provided by the > same software suite, for example, LLVM and Binutils. > > When we raise the minimal version of Clang/LLVM, we need to update > clang_min_version in scripts/cc-version.sh and also lld_min_version in > scripts/ld-version.sh. > > In fact, Kbuild can handle CC=clang and LD=ld.lld independently, and we > could manage their minimal version separately, but it does not make > much sense. > > Make scripts/tool-version.sh a central place of minimum tool versions > so that we do not need to touch multiple files. It would be better and self-explanatory if a script that provides the minimum version of a tool was actually called ... min_tool-version.sh or the like. Otherwise one might think it would give e.g. the current version of installed tools. Nicolas ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] kbuild: collect minimum tool versions into scripts/tool-version.sh 2021-03-04 22:10 ` Nicolas Pitre @ 2021-03-05 6:41 ` Masahiro Yamada 0 siblings, 0 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-05 6:41 UTC (permalink / raw) To: Nicolas Pitre Cc: Linux Kbuild mailing list, clang-built-linux, Nick Desaulniers, Nathan Chancellor, Linux Kernel Mailing List On Fri, Mar 5, 2021 at 7:10 AM Nicolas Pitre <nico@fluxnic.net> wrote: > > On Thu, 4 Mar 2021, Masahiro Yamada wrote: > > > The kernel build uses various tools, many of which are provided by the > > same software suite, for example, LLVM and Binutils. > > > > When we raise the minimal version of Clang/LLVM, we need to update > > clang_min_version in scripts/cc-version.sh and also lld_min_version in > > scripts/ld-version.sh. > > > > In fact, Kbuild can handle CC=clang and LD=ld.lld independently, and we > > could manage their minimal version separately, but it does not make > > much sense. > > > > Make scripts/tool-version.sh a central place of minimum tool versions > > so that we do not need to touch multiple files. > > It would be better and self-explanatory if a script that provides the > minimum version of a tool was actually called ... min_tool-version.sh or > the like. Otherwise one might think it would give e.g. the current > version of installed tools. > > > Nicolas You are right. I will rename it. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig 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-03 18:33 ` Masahiro Yamada 2021-03-04 0:28 ` Nathan Chancellor 2021-03-05 1:25 ` Nick Desaulniers 2021-03-03 18:33 ` [PATCH 4/4] kbuild: dwarf: use AS_VERSION instead of test_dwarf5_support.sh Masahiro Yamada ` (2 subsequent siblings) 4 siblings, 2 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-03 18:33 UTC (permalink / raw) To: linux-kbuild Cc: clang-built-linux, Nick Desaulniers, Masahiro Yamada, Nathan Chancellor, linux-kernel Documentation/process/changes.rst defines the minimum assembler version (binutils version), but we have never checked it in the build time. Kbuild never invokes 'as' directly because all assembly files in the kernel tree are *.S, hence must be preprocessed. I do not expect raw assembly source files (*.s) would be added to the kernel tree. Therefore, we always use $(CC) as the assembler driver, and commit aa824e0c962b ("kbuild: remove AS variable") removed 'AS'. However, we are still interested in the version of the assembler sitting behind. As usual, the --version option prints the version string. $ as --version | head -n 1 GNU assembler (GNU Binutils for Ubuntu) 2.35.1 But, we do not have $(AS). So, we can add the -Wa prefix so that $(CC) passes --version down to the backing assembler. $ gcc -Wa,--version | head -n 1 gcc: fatal error: no input files compilation terminated. OK, we need to input something to satisfy gcc. $ gcc -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 GNU assembler (GNU Binutils for Ubuntu) 2.35.1 The combination of Clang and GNU assembler works in the same way: $ clang -no-integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 GNU assembler (GNU Binutils for Ubuntu) 2.35.1 Clang with the integrated assembler fails like this: $ clang -integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 clang: error: unsupported argument '--version' to option 'Wa,' With all this in my mind, I implemented scripts/as-version.sh. $ scripts/as-version.sh gcc GNU 23501 $ scripts/as-version.sh clang -no-integrated-as GNU 23501 $ scripts/as-version.sh clang -integrated-as LLVM 0 Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- arch/Kconfig | 3 +- init/Kconfig | 12 +++++++ scripts/Kconfig.include | 6 ++++ scripts/as-version.sh | 77 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 2 deletions(-) create mode 100755 scripts/as-version.sh diff --git a/arch/Kconfig b/arch/Kconfig index 2af10ebe5ed0..d7214f4ae1f7 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -631,8 +631,7 @@ config ARCH_SUPPORTS_LTO_CLANG_THIN 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_IAS) -eq 1) + depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm) depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm) depends on ARCH_SUPPORTS_LTO_CLANG diff --git a/init/Kconfig b/init/Kconfig index 22946fe5ded9..f76e5a44e4fe 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -41,6 +41,18 @@ config CLANG_VERSION default $(cc-version) if CC_IS_CLANG default 0 +config AS_IS_GNU + def_bool $(success,test "$(as-name)" = GNU) + +config AS_IS_LLVM + def_bool $(success,test "$(as-name)" = LLVM) + +config AS_VERSION + int + # If it is integrated assembler, the version is the same as Clang's one. + default CLANG_VERSION if AS_IS_LLVM + default $(as-version) + config LD_IS_BFD def_bool $(success,test "$(ld-name)" = BFD) diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include index 58fdb5308725..0496efd6e117 100644 --- a/scripts/Kconfig.include +++ b/scripts/Kconfig.include @@ -45,6 +45,12 @@ $(error-if,$(success,test -z "$(cc-info)"),Sorry$(comma) this compiler is not su cc-name := $(shell,set -- $(cc-info) && echo $1) cc-version := $(shell,set -- $(cc-info) && echo $2) +# Get the assembler name, version, and error out if it is not supported. +as-info := $(shell,$(srctree)/scripts/as-version.sh $(CC) $(CLANG_FLAGS)) +$(error-if,$(success,test -z "$(as-info)"),Sorry$(comma) this assembler is not supported.) +as-name := $(shell,set -- $(as-info) && echo $1) +as-version := $(shell,set -- $(as-info) && echo $2) + # Get the linker name, version, and error out if it is not supported. ld-info := $(shell,$(srctree)/scripts/ld-version.sh $(LD)) $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supported.) diff --git a/scripts/as-version.sh b/scripts/as-version.sh new file mode 100755 index 000000000000..205d8b9fc4d4 --- /dev/null +++ b/scripts/as-version.sh @@ -0,0 +1,77 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0-only +# +# Print the assembler name and its version in a 5 or 6-digit form. +# Also, perform the minimum version check. +# (If it is the integrated assembler, return 0 as the version, and +# the version check is skipped.) + +set -e + +# Convert the version string x.y.z to a canonical 5 or 6-digit form. +get_canonical_version() +{ + IFS=. + set -- $1 + + # If the 2nd or 3rd field is missing, fill it with a zero. + # + # The 4th field, if present, is ignored. + # This occurs in development snapshots as in 2.35.1.20201116 + echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0})) +} + +orig_args="$@" + +# Get the first line of the --version output. +IFS=' +' +# Add 2>&1 to check both stdout and stderr. +# If the backing assembler is binutils, we get the version string in stdout. +# If it is clang's integrated assembler, we get the following error in stderr: +# clang: error: unsupported argument '--version' to option 'Wa,' +# To avoid the error message affected by locale, set LC_MESSAGES=C just in case. +set -- $(LC_MESSAGES=C "$@" -Wno-unused-command-line-argument -Wa,--version -c -x assembler /dev/null -o /dev/null 2>&1) +line="$1" + +if [ "$line" = "clang: error: unsupported argument '--version' to option 'Wa,'" ]; then + # For the intergrated assembler, we do not check the version here. + # It is the same as the clang version, and it has been already checked + # by scripts/cc-version.sh. + echo LLVM 0 + exit 0 +fi + +# Split the line on spaces. +IFS=' ' +set -- $line + +tool_version=$(dirname $0)/tool-version.sh + +if [ "$1" = GNU -a "$2" = assembler ]; then + shift $(($# - 1)) + version=$1 + min_version=$($tool_version binutils) + name=GNU +else + echo "$orig_args: unknown assembler invoked" >&2 + exit 1 +fi + +# Some distributions append a package release number, as in 2.34-4.fc32 +# Trim the hyphen and any characters that follow. +version=${version%-*} + +cversion=$(get_canonical_version $version) +min_cversion=$(get_canonical_version $min_version) + +if [ "$cversion" -lt "$min_cversion" ]; then + echo >&2 "***" + echo >&2 "*** Assembler is too old." + echo >&2 "*** Your $name assembler version: $version" + echo >&2 "*** Minimum $name assembler version: $min_version" + echo >&2 "***" + exit 1 +fi + +echo $name $cversion -- 2.27.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig 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 1 sibling, 0 replies; 20+ messages in thread From: Nathan Chancellor @ 2021-03-04 0:28 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, clang-built-linux, Nick Desaulniers, linux-kernel On Thu, Mar 04, 2021 at 03:33:32AM +0900, Masahiro Yamada wrote: > Documentation/process/changes.rst defines the minimum assembler version > (binutils version), but we have never checked it in the build time. > > Kbuild never invokes 'as' directly because all assembly files in the > kernel tree are *.S, hence must be preprocessed. I do not expect > raw assembly source files (*.s) would be added to the kernel tree. > > Therefore, we always use $(CC) as the assembler driver, and commit > aa824e0c962b ("kbuild: remove AS variable") removed 'AS'. However, > we are still interested in the version of the assembler sitting behind. > > As usual, the --version option prints the version string. > > $ as --version | head -n 1 > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > But, we do not have $(AS). So, we can add the -Wa prefix so that > $(CC) passes --version down to the backing assembler. > > $ gcc -Wa,--version | head -n 1 > gcc: fatal error: no input files > compilation terminated. > > OK, we need to input something to satisfy gcc. > > $ gcc -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > The combination of Clang and GNU assembler works in the same way: > > $ clang -no-integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > Clang with the integrated assembler fails like this: > > $ clang -integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > clang: error: unsupported argument '--version' to option 'Wa,' > > With all this in my mind, I implemented scripts/as-version.sh. > > $ scripts/as-version.sh gcc > GNU 23501 > $ scripts/as-version.sh clang -no-integrated-as > GNU 23501 > $ scripts/as-version.sh clang -integrated-as > LLVM 0 > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > arch/Kconfig | 3 +- > init/Kconfig | 12 +++++++ > scripts/Kconfig.include | 6 ++++ > scripts/as-version.sh | 77 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 96 insertions(+), 2 deletions(-) > create mode 100755 scripts/as-version.sh > > diff --git a/arch/Kconfig b/arch/Kconfig > index 2af10ebe5ed0..d7214f4ae1f7 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -631,8 +631,7 @@ config ARCH_SUPPORTS_LTO_CLANG_THIN > 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_IAS) -eq 1) > + depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM > depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm) > depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm) > depends on ARCH_SUPPORTS_LTO_CLANG > diff --git a/init/Kconfig b/init/Kconfig > index 22946fe5ded9..f76e5a44e4fe 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -41,6 +41,18 @@ config CLANG_VERSION > default $(cc-version) if CC_IS_CLANG > default 0 > > +config AS_IS_GNU > + def_bool $(success,test "$(as-name)" = GNU) > + > +config AS_IS_LLVM > + def_bool $(success,test "$(as-name)" = LLVM) > + > +config AS_VERSION > + int > + # If it is integrated assembler, the version is the same as Clang's one. > + default CLANG_VERSION if AS_IS_LLVM > + default $(as-version) > + > config LD_IS_BFD > def_bool $(success,test "$(ld-name)" = BFD) > > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > index 58fdb5308725..0496efd6e117 100644 > --- a/scripts/Kconfig.include > +++ b/scripts/Kconfig.include > @@ -45,6 +45,12 @@ $(error-if,$(success,test -z "$(cc-info)"),Sorry$(comma) this compiler is not su > cc-name := $(shell,set -- $(cc-info) && echo $1) > cc-version := $(shell,set -- $(cc-info) && echo $2) > > +# Get the assembler name, version, and error out if it is not supported. > +as-info := $(shell,$(srctree)/scripts/as-version.sh $(CC) $(CLANG_FLAGS)) > +$(error-if,$(success,test -z "$(as-info)"),Sorry$(comma) this assembler is not supported.) > +as-name := $(shell,set -- $(as-info) && echo $1) > +as-version := $(shell,set -- $(as-info) && echo $2) > + > # Get the linker name, version, and error out if it is not supported. > ld-info := $(shell,$(srctree)/scripts/ld-version.sh $(LD)) > $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supported.) > diff --git a/scripts/as-version.sh b/scripts/as-version.sh > new file mode 100755 > index 000000000000..205d8b9fc4d4 > --- /dev/null > +++ b/scripts/as-version.sh > @@ -0,0 +1,77 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Print the assembler name and its version in a 5 or 6-digit form. > +# Also, perform the minimum version check. > +# (If it is the integrated assembler, return 0 as the version, and > +# the version check is skipped.) > + > +set -e > + > +# Convert the version string x.y.z to a canonical 5 or 6-digit form. > +get_canonical_version() > +{ > + IFS=. > + set -- $1 > + > + # If the 2nd or 3rd field is missing, fill it with a zero. > + # > + # The 4th field, if present, is ignored. > + # This occurs in development snapshots as in 2.35.1.20201116 > + echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0})) > +} > + > +orig_args="$@" > + > +# Get the first line of the --version output. > +IFS=' > +' > +# Add 2>&1 to check both stdout and stderr. > +# If the backing assembler is binutils, we get the version string in stdout. > +# If it is clang's integrated assembler, we get the following error in stderr: > +# clang: error: unsupported argument '--version' to option 'Wa,' > +# To avoid the error message affected by locale, set LC_MESSAGES=C just in case. > +set -- $(LC_MESSAGES=C "$@" -Wno-unused-command-line-argument -Wa,--version -c -x assembler /dev/null -o /dev/null 2>&1) > +line="$1" > + > +if [ "$line" = "clang: error: unsupported argument '--version' to option 'Wa,'" ]; then It looks like Debian and Ubuntu do something interesting with their clang packaging. Normally, "clang" is a symlink to "clang-#" in the same folder. In my folder from just running "ninja install" $ ls -l clang clang-13 lrwxrwxrwx 8 nathan 2 Mar 20:58 clang -> clang-13 .rwxr-xr-x 93M nathan 2 Mar 20:58 clang-13 The same thing is true for Arch Linux: $ ls -l /usr/bin/{clang,clang-11} lrwxrwxrwx 8 root 17 Feb 8:54 /usr/bin/clang -> clang-11 .rwxr-xr-x 145k root 17 Feb 8:54 /usr/bin/clang-11 As a result, this does not quite work. $ LC_MESSAGES=C /usr/bin/clang -Wno-unused-command-line-argument -Wa,--version -c -x assembler /dev/null -o /dev/null clang-11: error: unsupported argument '--version' to option 'Wa,' I am not sure what the cleanest fix would be. The rest of the patch looks okay to me. Cheers, Nathan > + # For the intergrated assembler, we do not check the version here. > + # It is the same as the clang version, and it has been already checked > + # by scripts/cc-version.sh. > + echo LLVM 0 > + exit 0 > +fi > + > +# Split the line on spaces. > +IFS=' ' > +set -- $line > + > +tool_version=$(dirname $0)/tool-version.sh > + > +if [ "$1" = GNU -a "$2" = assembler ]; then > + shift $(($# - 1)) > + version=$1 > + min_version=$($tool_version binutils) > + name=GNU > +else > + echo "$orig_args: unknown assembler invoked" >&2 > + exit 1 > +fi > + > +# Some distributions append a package release number, as in 2.34-4.fc32 > +# Trim the hyphen and any characters that follow. > +version=${version%-*} > + > +cversion=$(get_canonical_version $version) > +min_cversion=$(get_canonical_version $min_version) > + > +if [ "$cversion" -lt "$min_cversion" ]; then > + echo >&2 "***" > + echo >&2 "*** Assembler is too old." > + echo >&2 "*** Your $name assembler version: $version" > + echo >&2 "*** Minimum $name assembler version: $min_version" > + echo >&2 "***" > + exit 1 > +fi > + > +echo $name $cversion > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig 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 1 sibling, 1 reply; 20+ messages in thread From: Nick Desaulniers @ 2021-03-05 1:25 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kbuild mailing list, clang-built-linux, Nathan Chancellor, LKML, Jian Cai On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Documentation/process/changes.rst defines the minimum assembler version > (binutils version), but we have never checked it in the build time. > > Kbuild never invokes 'as' directly because all assembly files in the > kernel tree are *.S, hence must be preprocessed. I do not expect > raw assembly source files (*.s) would be added to the kernel tree. > > Therefore, we always use $(CC) as the assembler driver, and commit > aa824e0c962b ("kbuild: remove AS variable") removed 'AS'. However, > we are still interested in the version of the assembler sitting behind. > > As usual, the --version option prints the version string. > > $ as --version | head -n 1 > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > But, we do not have $(AS). So, we can add the -Wa prefix so that > $(CC) passes --version down to the backing assembler. > > $ gcc -Wa,--version | head -n 1 > gcc: fatal error: no input files > compilation terminated. > > OK, we need to input something to satisfy gcc. > > $ gcc -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > The combination of Clang and GNU assembler works in the same way: > > $ clang -no-integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > Clang with the integrated assembler fails like this: > > $ clang -integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > clang: error: unsupported argument '--version' to option 'Wa,' Was this a feature request to "please implement -Wa,--version for clang?" :-P https://github.com/ClangBuiltLinux/linux/issues/1320 > > With all this in my mind, I implemented scripts/as-version.sh. > > $ scripts/as-version.sh gcc > GNU 23501 > $ scripts/as-version.sh clang -no-integrated-as > GNU 23501 > $ scripts/as-version.sh clang -integrated-as > LLVM 0 > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > arch/Kconfig | 3 +- > init/Kconfig | 12 +++++++ > scripts/Kconfig.include | 6 ++++ > scripts/as-version.sh | 77 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 96 insertions(+), 2 deletions(-) > create mode 100755 scripts/as-version.sh > > diff --git a/arch/Kconfig b/arch/Kconfig > index 2af10ebe5ed0..d7214f4ae1f7 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -631,8 +631,7 @@ config ARCH_SUPPORTS_LTO_CLANG_THIN > 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_IAS) -eq 1) > + depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM > depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm) > depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm) > depends on ARCH_SUPPORTS_LTO_CLANG > diff --git a/init/Kconfig b/init/Kconfig > index 22946fe5ded9..f76e5a44e4fe 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -41,6 +41,18 @@ config CLANG_VERSION > default $(cc-version) if CC_IS_CLANG > default 0 > > +config AS_IS_GNU > + def_bool $(success,test "$(as-name)" = GNU) > + > +config AS_IS_LLVM > + def_bool $(success,test "$(as-name)" = LLVM) > + > +config AS_VERSION > + int > + # If it is integrated assembler, the version is the same as Clang's one. > + default CLANG_VERSION if AS_IS_LLVM > + default $(as-version) > + > config LD_IS_BFD > def_bool $(success,test "$(ld-name)" = BFD) > > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > index 58fdb5308725..0496efd6e117 100644 > --- a/scripts/Kconfig.include > +++ b/scripts/Kconfig.include > @@ -45,6 +45,12 @@ $(error-if,$(success,test -z "$(cc-info)"),Sorry$(comma) this compiler is not su > cc-name := $(shell,set -- $(cc-info) && echo $1) > cc-version := $(shell,set -- $(cc-info) && echo $2) > > +# Get the assembler name, version, and error out if it is not supported. > +as-info := $(shell,$(srctree)/scripts/as-version.sh $(CC) $(CLANG_FLAGS)) > +$(error-if,$(success,test -z "$(as-info)"),Sorry$(comma) this assembler is not supported.) > +as-name := $(shell,set -- $(as-info) && echo $1) > +as-version := $(shell,set -- $(as-info) && echo $2) > + > # Get the linker name, version, and error out if it is not supported. > ld-info := $(shell,$(srctree)/scripts/ld-version.sh $(LD)) > $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supported.) > diff --git a/scripts/as-version.sh b/scripts/as-version.sh > new file mode 100755 > index 000000000000..205d8b9fc4d4 > --- /dev/null > +++ b/scripts/as-version.sh > @@ -0,0 +1,77 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# Print the assembler name and its version in a 5 or 6-digit form. > +# Also, perform the minimum version check. > +# (If it is the integrated assembler, return 0 as the version, and > +# the version check is skipped.) > + > +set -e > + > +# Convert the version string x.y.z to a canonical 5 or 6-digit form. > +get_canonical_version() > +{ > + IFS=. > + set -- $1 > + > + # If the 2nd or 3rd field is missing, fill it with a zero. > + # > + # The 4th field, if present, is ignored. > + # This occurs in development snapshots as in 2.35.1.20201116 > + echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0})) > +} > + > +orig_args="$@" > + > +# Get the first line of the --version output. > +IFS=' > +' > +# Add 2>&1 to check both stdout and stderr. > +# If the backing assembler is binutils, we get the version string in stdout. > +# If it is clang's integrated assembler, we get the following error in stderr: > +# clang: error: unsupported argument '--version' to option 'Wa,' > +# To avoid the error message affected by locale, set LC_MESSAGES=C just in case. > +set -- $(LC_MESSAGES=C "$@" -Wno-unused-command-line-argument -Wa,--version -c -x assembler /dev/null -o /dev/null 2>&1) > +line="$1" > + > +if [ "$line" = "clang: error: unsupported argument '--version' to option 'Wa,'" ]; then Checking the precise error message is too brittle; what if it changes? Why not check the return code a la cc-option and friends? Is checking the return code of a subshell an issue here? > + # For the intergrated assembler, we do not check the version here. s/intergrated/integrated/ > + # It is the same as the clang version, and it has been already checked > + # by scripts/cc-version.sh. > + echo LLVM 0 > + exit 0 > +fi > + > +# Split the line on spaces. > +IFS=' ' > +set -- $line > + > +tool_version=$(dirname $0)/tool-version.sh > + > +if [ "$1" = GNU -a "$2" = assembler ]; then > + shift $(($# - 1)) > + version=$1 > + min_version=$($tool_version binutils) > + name=GNU > +else > + echo "$orig_args: unknown assembler invoked" >&2 > + exit 1 > +fi > + > +# Some distributions append a package release number, as in 2.34-4.fc32 > +# Trim the hyphen and any characters that follow. > +version=${version%-*} > + > +cversion=$(get_canonical_version $version) > +min_cversion=$(get_canonical_version $min_version) > + > +if [ "$cversion" -lt "$min_cversion" ]; then > + echo >&2 "***" > + echo >&2 "*** Assembler is too old." > + echo >&2 "*** Your $name assembler version: $version" > + echo >&2 "*** Minimum $name assembler version: $min_version" > + echo >&2 "***" > + exit 1 > +fi > + > +echo $name $cversion > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig 2021-03-05 1:25 ` Nick Desaulniers @ 2021-03-05 17:48 ` Masahiro Yamada 2021-03-07 4:42 ` Nathan Chancellor 0 siblings, 1 reply; 20+ messages in thread From: Masahiro Yamada @ 2021-03-05 17:48 UTC (permalink / raw) To: Nick Desaulniers Cc: Linux Kbuild mailing list, clang-built-linux, Nathan Chancellor, LKML, Jian Cai On Fri, Mar 5, 2021 at 10:26 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Documentation/process/changes.rst defines the minimum assembler version > > (binutils version), but we have never checked it in the build time. > > > > Kbuild never invokes 'as' directly because all assembly files in the > > kernel tree are *.S, hence must be preprocessed. I do not expect > > raw assembly source files (*.s) would be added to the kernel tree. > > > > Therefore, we always use $(CC) as the assembler driver, and commit > > aa824e0c962b ("kbuild: remove AS variable") removed 'AS'. However, > > we are still interested in the version of the assembler sitting behind. > > > > As usual, the --version option prints the version string. > > > > $ as --version | head -n 1 > > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > > > But, we do not have $(AS). So, we can add the -Wa prefix so that > > $(CC) passes --version down to the backing assembler. > > > > $ gcc -Wa,--version | head -n 1 > > gcc: fatal error: no input files > > compilation terminated. > > > > OK, we need to input something to satisfy gcc. > > > > $ gcc -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > > > The combination of Clang and GNU assembler works in the same way: > > > > $ clang -no-integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > > > Clang with the integrated assembler fails like this: > > > > $ clang -integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > > clang: error: unsupported argument '--version' to option 'Wa,' > > Was this a feature request to "please implement -Wa,--version for clang?" :-P > https://github.com/ClangBuiltLinux/linux/issues/1320 > > > > > With all this in my mind, I implemented scripts/as-version.sh. > > > > $ scripts/as-version.sh gcc > > GNU 23501 > > $ scripts/as-version.sh clang -no-integrated-as > > GNU 23501 > > $ scripts/as-version.sh clang -integrated-as > > LLVM 0 > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > arch/Kconfig | 3 +- > > init/Kconfig | 12 +++++++ > > scripts/Kconfig.include | 6 ++++ > > scripts/as-version.sh | 77 +++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 96 insertions(+), 2 deletions(-) > > create mode 100755 scripts/as-version.sh > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index 2af10ebe5ed0..d7214f4ae1f7 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -631,8 +631,7 @@ config ARCH_SUPPORTS_LTO_CLANG_THIN > > 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_IAS) -eq 1) > > + depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM > > depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm) > > depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm) > > depends on ARCH_SUPPORTS_LTO_CLANG > > diff --git a/init/Kconfig b/init/Kconfig > > index 22946fe5ded9..f76e5a44e4fe 100644 > > --- a/init/Kconfig > > +++ b/init/Kconfig > > @@ -41,6 +41,18 @@ config CLANG_VERSION > > default $(cc-version) if CC_IS_CLANG > > default 0 > > > > +config AS_IS_GNU > > + def_bool $(success,test "$(as-name)" = GNU) > > + > > +config AS_IS_LLVM > > + def_bool $(success,test "$(as-name)" = LLVM) > > + > > +config AS_VERSION > > + int > > + # If it is integrated assembler, the version is the same as Clang's one. > > + default CLANG_VERSION if AS_IS_LLVM > > + default $(as-version) > > + > > config LD_IS_BFD > > def_bool $(success,test "$(ld-name)" = BFD) > > > > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > > index 58fdb5308725..0496efd6e117 100644 > > --- a/scripts/Kconfig.include > > +++ b/scripts/Kconfig.include > > @@ -45,6 +45,12 @@ $(error-if,$(success,test -z "$(cc-info)"),Sorry$(comma) this compiler is not su > > cc-name := $(shell,set -- $(cc-info) && echo $1) > > cc-version := $(shell,set -- $(cc-info) && echo $2) > > > > +# Get the assembler name, version, and error out if it is not supported. > > +as-info := $(shell,$(srctree)/scripts/as-version.sh $(CC) $(CLANG_FLAGS)) > > +$(error-if,$(success,test -z "$(as-info)"),Sorry$(comma) this assembler is not supported.) > > +as-name := $(shell,set -- $(as-info) && echo $1) > > +as-version := $(shell,set -- $(as-info) && echo $2) > > + > > # Get the linker name, version, and error out if it is not supported. > > ld-info := $(shell,$(srctree)/scripts/ld-version.sh $(LD)) > > $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supported.) > > diff --git a/scripts/as-version.sh b/scripts/as-version.sh > > new file mode 100755 > > index 000000000000..205d8b9fc4d4 > > --- /dev/null > > +++ b/scripts/as-version.sh > > @@ -0,0 +1,77 @@ > > +#!/bin/sh > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# Print the assembler name and its version in a 5 or 6-digit form. > > +# Also, perform the minimum version check. > > +# (If it is the integrated assembler, return 0 as the version, and > > +# the version check is skipped.) > > + > > +set -e > > + > > +# Convert the version string x.y.z to a canonical 5 or 6-digit form. > > +get_canonical_version() > > +{ > > + IFS=. > > + set -- $1 > > + > > + # If the 2nd or 3rd field is missing, fill it with a zero. > > + # > > + # The 4th field, if present, is ignored. > > + # This occurs in development snapshots as in 2.35.1.20201116 > > + echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0})) > > +} > > + > > +orig_args="$@" > > + > > +# Get the first line of the --version output. > > +IFS=' > > +' > > +# Add 2>&1 to check both stdout and stderr. > > +# If the backing assembler is binutils, we get the version string in stdout. > > +# If it is clang's integrated assembler, we get the following error in stderr: > > +# clang: error: unsupported argument '--version' to option 'Wa,' > > +# To avoid the error message affected by locale, set LC_MESSAGES=C just in case. > > +set -- $(LC_MESSAGES=C "$@" -Wno-unused-command-line-argument -Wa,--version -c -x assembler /dev/null -o /dev/null 2>&1) > > +line="$1" > > + > > +if [ "$line" = "clang: error: unsupported argument '--version' to option 'Wa,'" ]; then > > Checking the precise error message is too brittle; what if it changes? > Why not check the return code a la cc-option and friends? Is checking > the return code of a subshell an issue here? As Nathan pointed out, this is fragile. I just tried my best to make this script work stand-alone. With the only exit code chec, scripts/as-version.sh false would return LLVM, which is false-positive. Probably this is not a big deal since completely wrong input was blocked by scripts/cc-version.sh If we give up making this script stand-alone, another idea is to check LLVM_IAS=1, which is passed from the top Makefile. Yet another idea is to explicitly pass -integrated-as as CLANG_FLAGS. Then, this script can parse the presence of -integrated-as. BTW, is there any guarantee that the integrated assembler is always enabled by default? Or, is it dependent on the configuration? The top Makefile adds -no-integrated-as if LLVM_IAS != 1, but adds nothing if LLVM_IAS == 1. So, -integrated-as must be always default. > > > + # For the intergrated assembler, we do not check the version here. > > s/intergrated/integrated/ > > > + # It is the same as the clang version, and it has been already checked > > + # by scripts/cc-version.sh. > > + echo LLVM 0 > > + exit 0 > > +fi > > + > > +# Split the line on spaces. > > +IFS=' ' > > +set -- $line > > + > > +tool_version=$(dirname $0)/tool-version.sh > > + > > +if [ "$1" = GNU -a "$2" = assembler ]; then > > + shift $(($# - 1)) > > + version=$1 > > + min_version=$($tool_version binutils) > > + name=GNU > > +else > > + echo "$orig_args: unknown assembler invoked" >&2 > > + exit 1 > > +fi > > + > > +# Some distributions append a package release number, as in 2.34-4.fc32 > > +# Trim the hyphen and any characters that follow. > > +version=${version%-*} > > + > > +cversion=$(get_canonical_version $version) > > +min_cversion=$(get_canonical_version $min_version) > > + > > +if [ "$cversion" -lt "$min_cversion" ]; then > > + echo >&2 "***" > > + echo >&2 "*** Assembler is too old." > > + echo >&2 "*** Your $name assembler version: $version" > > + echo >&2 "*** Minimum $name assembler version: $min_version" > > + echo >&2 "***" > > + exit 1 > > +fi > > + > > +echo $name $cversion > > -- > > 2.27.0 > > > > > -- > Thanks, > ~Nick Desaulniers -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig 2021-03-05 17:48 ` Masahiro Yamada @ 2021-03-07 4:42 ` Nathan Chancellor 0 siblings, 0 replies; 20+ messages in thread From: Nathan Chancellor @ 2021-03-07 4:42 UTC (permalink / raw) To: Masahiro Yamada Cc: Nick Desaulniers, Linux Kbuild mailing list, clang-built-linux, LKML, Jian Cai On Sat, Mar 06, 2021 at 02:48:38AM +0900, Masahiro Yamada wrote: > On Fri, Mar 5, 2021 at 10:26 AM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > Documentation/process/changes.rst defines the minimum assembler version > > > (binutils version), but we have never checked it in the build time. > > > > > > Kbuild never invokes 'as' directly because all assembly files in the > > > kernel tree are *.S, hence must be preprocessed. I do not expect > > > raw assembly source files (*.s) would be added to the kernel tree. > > > > > > Therefore, we always use $(CC) as the assembler driver, and commit > > > aa824e0c962b ("kbuild: remove AS variable") removed 'AS'. However, > > > we are still interested in the version of the assembler sitting behind. > > > > > > As usual, the --version option prints the version string. > > > > > > $ as --version | head -n 1 > > > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > > > > > But, we do not have $(AS). So, we can add the -Wa prefix so that > > > $(CC) passes --version down to the backing assembler. > > > > > > $ gcc -Wa,--version | head -n 1 > > > gcc: fatal error: no input files > > > compilation terminated. > > > > > > OK, we need to input something to satisfy gcc. > > > > > > $ gcc -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > > > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > > > > > The combination of Clang and GNU assembler works in the same way: > > > > > > $ clang -no-integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > > > GNU assembler (GNU Binutils for Ubuntu) 2.35.1 > > > > > > Clang with the integrated assembler fails like this: > > > > > > $ clang -integrated-as -Wa,--version -c -x assembler /dev/null -o /dev/null | head -n 1 > > > clang: error: unsupported argument '--version' to option 'Wa,' > > > > Was this a feature request to "please implement -Wa,--version for clang?" :-P > > https://github.com/ClangBuiltLinux/linux/issues/1320 > > > > > > > > With all this in my mind, I implemented scripts/as-version.sh. > > > > > > $ scripts/as-version.sh gcc > > > GNU 23501 > > > $ scripts/as-version.sh clang -no-integrated-as > > > GNU 23501 > > > $ scripts/as-version.sh clang -integrated-as > > > LLVM 0 > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > --- > > > > > > arch/Kconfig | 3 +- > > > init/Kconfig | 12 +++++++ > > > scripts/Kconfig.include | 6 ++++ > > > scripts/as-version.sh | 77 +++++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 96 insertions(+), 2 deletions(-) > > > create mode 100755 scripts/as-version.sh > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > > index 2af10ebe5ed0..d7214f4ae1f7 100644 > > > --- a/arch/Kconfig > > > +++ b/arch/Kconfig > > > @@ -631,8 +631,7 @@ config ARCH_SUPPORTS_LTO_CLANG_THIN > > > 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_IAS) -eq 1) > > > + depends on CC_IS_CLANG && CLANG_VERSION >= 110000 && LD_IS_LLD && AS_IS_LLVM > > > depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm) > > > depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm) > > > depends on ARCH_SUPPORTS_LTO_CLANG > > > diff --git a/init/Kconfig b/init/Kconfig > > > index 22946fe5ded9..f76e5a44e4fe 100644 > > > --- a/init/Kconfig > > > +++ b/init/Kconfig > > > @@ -41,6 +41,18 @@ config CLANG_VERSION > > > default $(cc-version) if CC_IS_CLANG > > > default 0 > > > > > > +config AS_IS_GNU > > > + def_bool $(success,test "$(as-name)" = GNU) > > > + > > > +config AS_IS_LLVM > > > + def_bool $(success,test "$(as-name)" = LLVM) > > > + > > > +config AS_VERSION > > > + int > > > + # If it is integrated assembler, the version is the same as Clang's one. > > > + default CLANG_VERSION if AS_IS_LLVM > > > + default $(as-version) > > > + > > > config LD_IS_BFD > > > def_bool $(success,test "$(ld-name)" = BFD) > > > > > > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > > > index 58fdb5308725..0496efd6e117 100644 > > > --- a/scripts/Kconfig.include > > > +++ b/scripts/Kconfig.include > > > @@ -45,6 +45,12 @@ $(error-if,$(success,test -z "$(cc-info)"),Sorry$(comma) this compiler is not su > > > cc-name := $(shell,set -- $(cc-info) && echo $1) > > > cc-version := $(shell,set -- $(cc-info) && echo $2) > > > > > > +# Get the assembler name, version, and error out if it is not supported. > > > +as-info := $(shell,$(srctree)/scripts/as-version.sh $(CC) $(CLANG_FLAGS)) > > > +$(error-if,$(success,test -z "$(as-info)"),Sorry$(comma) this assembler is not supported.) > > > +as-name := $(shell,set -- $(as-info) && echo $1) > > > +as-version := $(shell,set -- $(as-info) && echo $2) > > > + > > > # Get the linker name, version, and error out if it is not supported. > > > ld-info := $(shell,$(srctree)/scripts/ld-version.sh $(LD)) > > > $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supported.) > > > diff --git a/scripts/as-version.sh b/scripts/as-version.sh > > > new file mode 100755 > > > index 000000000000..205d8b9fc4d4 > > > --- /dev/null > > > +++ b/scripts/as-version.sh > > > @@ -0,0 +1,77 @@ > > > +#!/bin/sh > > > +# SPDX-License-Identifier: GPL-2.0-only > > > +# > > > +# Print the assembler name and its version in a 5 or 6-digit form. > > > +# Also, perform the minimum version check. > > > +# (If it is the integrated assembler, return 0 as the version, and > > > +# the version check is skipped.) > > > + > > > +set -e > > > + > > > +# Convert the version string x.y.z to a canonical 5 or 6-digit form. > > > +get_canonical_version() > > > +{ > > > + IFS=. > > > + set -- $1 > > > + > > > + # If the 2nd or 3rd field is missing, fill it with a zero. > > > + # > > > + # The 4th field, if present, is ignored. > > > + # This occurs in development snapshots as in 2.35.1.20201116 > > > + echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0})) > > > +} > > > + > > > +orig_args="$@" > > > + > > > +# Get the first line of the --version output. > > > +IFS=' > > > +' > > > +# Add 2>&1 to check both stdout and stderr. > > > +# If the backing assembler is binutils, we get the version string in stdout. > > > +# If it is clang's integrated assembler, we get the following error in stderr: > > > +# clang: error: unsupported argument '--version' to option 'Wa,' > > > +# To avoid the error message affected by locale, set LC_MESSAGES=C just in case. > > > +set -- $(LC_MESSAGES=C "$@" -Wno-unused-command-line-argument -Wa,--version -c -x assembler /dev/null -o /dev/null 2>&1) > > > +line="$1" > > > + > > > +if [ "$line" = "clang: error: unsupported argument '--version' to option 'Wa,'" ]; then > > > > Checking the precise error message is too brittle; what if it changes? > > Why not check the return code a la cc-option and friends? Is checking > > the return code of a subshell an issue here? > > As Nathan pointed out, this is fragile. > > I just tried my best to make this script work stand-alone. > > With the only exit code chec, > > scripts/as-version.sh false > > would return LLVM, which is false-positive. > Probably this is not a big deal > since completely wrong input was blocked by > scripts/cc-version.sh > > > If we give up making this script stand-alone, > another idea is to check LLVM_IAS=1, which > is passed from the top Makefile. > > Yet another idea is to explicitly pass > -integrated-as as CLANG_FLAGS. > Then, this script can parse the presence > of -integrated-as. This is probably the easiest thing to do for all versions of LLVM (adding -Wa,--version support to the integrated assembler as Nick suggests only fixes future versions of LLVM) and goes along with the "explicit is better than implicit" policy, which I tend to align with. It makes easy to quickly see what assembler was used when looking at a clang invocation as well. > BTW, is there any guarantee that the integrated assembler > is always enabled by default? Or, is it dependent on > the configuration? There is a virtual method called IsIntegratedAssemblerDefault(), which defaults to false but is overridden to true by various targets. For all of the architectures/targets that the kernel cares about, this appears to be true. https://github.com/llvm/llvm-project/blob/45f949ee469f3e59d30a88c97aa7c1139069b261/clang/lib/Driver/ToolChains/Gnu.cpp#L2738-L2774 Cheers, Nathan > The top Makefile adds -no-integrated-as if LLVM_IAS != 1, > but adds nothing if LLVM_IAS == 1. > So, -integrated-as must be always default. > > > > > > > > > > > > + # For the intergrated assembler, we do not check the version here. > > > > s/intergrated/integrated/ > > > > > + # It is the same as the clang version, and it has been already checked > > > + # by scripts/cc-version.sh. > > > + echo LLVM 0 > > > + exit 0 > > > +fi > > > + > > > +# Split the line on spaces. > > > +IFS=' ' > > > +set -- $line > > > + > > > +tool_version=$(dirname $0)/tool-version.sh > > > + > > > +if [ "$1" = GNU -a "$2" = assembler ]; then > > > + shift $(($# - 1)) > > > + version=$1 > > > + min_version=$($tool_version binutils) > > > + name=GNU > > > +else > > > + echo "$orig_args: unknown assembler invoked" >&2 > > > + exit 1 > > > +fi > > > + > > > +# Some distributions append a package release number, as in 2.34-4.fc32 > > > +# Trim the hyphen and any characters that follow. > > > +version=${version%-*} > > > + > > > +cversion=$(get_canonical_version $version) > > > +min_cversion=$(get_canonical_version $min_version) > > > + > > > +if [ "$cversion" -lt "$min_cversion" ]; then > > > + echo >&2 "***" > > > + echo >&2 "*** Assembler is too old." > > > + echo >&2 "*** Your $name assembler version: $version" > > > + echo >&2 "*** Minimum $name assembler version: $min_version" > > > + echo >&2 "***" > > > + exit 1 > > > +fi > > > + > > > +echo $name $cversion > > > -- > > > 2.27.0 > > > > > > > > > -- > > Thanks, > > ~Nick Desaulniers > > > > -- > Best Regards > Masahiro Yamada ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/4] kbuild: dwarf: use AS_VERSION instead of test_dwarf5_support.sh 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-03 18:33 ` [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig Masahiro Yamada @ 2021-03-03 18:33 ` Masahiro Yamada 2021-03-03 20:44 ` Nick Desaulniers 2021-03-04 0:30 ` Nathan Chancellor 2021-03-03 20:47 ` [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Nick Desaulniers 2021-03-03 23:37 ` Nathan Chancellor 4 siblings, 2 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-03 18:33 UTC (permalink / raw) To: linux-kbuild Cc: clang-built-linux, Nick Desaulniers, Masahiro Yamada, Nathan Chancellor, linux-kernel The test code in scripts/test_dwarf5_support.sh is somewhat difficult to understand, but after all, we want to check binutils >= 2.35.2 From the former discussion, the requrement for generating DRAWF v5 from C code is as follows: - gcc + binutils as -> requires gcc 5.0+ (but 7.0+ for full support) - clang + binutils as -> requires binutils 2.35.2+ - clang + integrated as -> OK Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- lib/Kconfig.debug | 3 +-- scripts/test_dwarf5_support.sh | 8 -------- 2 files changed, 1 insertion(+), 10 deletions(-) delete mode 100755 scripts/test_dwarf5_support.sh diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 2779c29d9981..f3337a38925d 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -284,8 +284,7 @@ config DEBUG_INFO_DWARF4 config DEBUG_INFO_DWARF5 bool "Generate DWARF Version 5 debuginfo" - depends on GCC_VERSION >= 50000 || CC_IS_CLANG - depends on CC_IS_GCC || $(success,$(srctree)/scripts/test_dwarf5_support.sh $(CC) $(CLANG_FLAGS)) + depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && AS_IS_GNU && AS_VERSION >= 23502) || (CC_IS_CLANG && AS_IS_LLVM) depends on !DEBUG_INFO_BTF help Generate DWARF v5 debug info. Requires binutils 2.35.2, gcc 5.0+ (gcc diff --git a/scripts/test_dwarf5_support.sh b/scripts/test_dwarf5_support.sh deleted file mode 100755 index c46e2456b47a..000000000000 --- a/scripts/test_dwarf5_support.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0 - -# Test that the assembler doesn't need -Wa,-gdwarf-5 when presented with DWARF -# v5 input, such as `.file 0` and `md5 0x00`. Should be fixed in GNU binutils -# 2.35.2. https://sourceware.org/bugzilla/show_bug.cgi?id=25611 -echo '.file 0 "filename" md5 0x7a0b65214090b6693bd1dc24dd248245' | \ - $* -gdwarf-5 -Wno-unused-command-line-argument -c -x assembler -o /dev/null - -- 2.27.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] kbuild: dwarf: use AS_VERSION instead of test_dwarf5_support.sh 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 1 sibling, 1 reply; 20+ messages in thread From: Nick Desaulniers @ 2021-03-03 20:44 UTC (permalink / raw) To: Masahiro Yamada Cc: Linux Kbuild mailing list, clang-built-linux, Nathan Chancellor, LKML On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > The test code in scripts/test_dwarf5_support.sh is somewhat difficult > to understand, but after all, we want to check binutils >= 2.35.2 > > From the former discussion, the requrement for generating DRAWF v5 from ^typos: s/requrement/requirement, s/DRAWF/DWARF (in vim you can `:set spell` (`:set nospell` to disable), there's probably a nice way to auto set this on buffer entry for a commit message) > C code is as follows: > > - gcc + binutils as -> requires gcc 5.0+ (but 7.0+ for full support) > - clang + binutils as -> requires binutils 2.35.2+ > - clang + integrated as -> OK Yes. Thanks for the patch. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > lib/Kconfig.debug | 3 +-- > scripts/test_dwarf5_support.sh | 8 -------- > 2 files changed, 1 insertion(+), 10 deletions(-) > delete mode 100755 scripts/test_dwarf5_support.sh > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 2779c29d9981..f3337a38925d 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -284,8 +284,7 @@ config DEBUG_INFO_DWARF4 > > config DEBUG_INFO_DWARF5 > bool "Generate DWARF Version 5 debuginfo" > - depends on GCC_VERSION >= 50000 || CC_IS_CLANG > - depends on CC_IS_GCC || $(success,$(srctree)/scripts/test_dwarf5_support.sh $(CC) $(CLANG_FLAGS)) > + depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && AS_IS_GNU && AS_VERSION >= 23502) || (CC_IS_CLANG && AS_IS_LLVM) Would this be more concise as: + depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502)) > depends on !DEBUG_INFO_BTF > help > Generate DWARF v5 debug info. Requires binutils 2.35.2, gcc 5.0+ (gcc > diff --git a/scripts/test_dwarf5_support.sh b/scripts/test_dwarf5_support.sh > deleted file mode 100755 > index c46e2456b47a..000000000000 > --- a/scripts/test_dwarf5_support.sh > +++ /dev/null > @@ -1,8 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > - > -# Test that the assembler doesn't need -Wa,-gdwarf-5 when presented with DWARF > -# v5 input, such as `.file 0` and `md5 0x00`. Should be fixed in GNU binutils > -# 2.35.2. https://sourceware.org/bugzilla/show_bug.cgi?id=25611 > -echo '.file 0 "filename" md5 0x7a0b65214090b6693bd1dc24dd248245' | \ > - $* -gdwarf-5 -Wno-unused-command-line-argument -c -x assembler -o /dev/null - > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] kbuild: dwarf: use AS_VERSION instead of test_dwarf5_support.sh 2021-03-03 20:44 ` Nick Desaulniers @ 2021-03-12 17:18 ` Masahiro Yamada 0 siblings, 0 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-12 17:18 UTC (permalink / raw) To: Nick Desaulniers Cc: Linux Kbuild mailing list, clang-built-linux, Nathan Chancellor, LKML On Thu, Mar 4, 2021 at 5:44 AM 'Nick Desaulniers' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > The test code in scripts/test_dwarf5_support.sh is somewhat difficult > > to understand, but after all, we want to check binutils >= 2.35.2 > > > > From the former discussion, the requrement for generating DRAWF v5 from > > ^typos: s/requrement/requirement, s/DRAWF/DWARF > > (in vim you can `:set spell` (`:set nospell` to disable), there's > probably a nice way to auto set this on buffer entry for a commit > message) I cannot be accustomed to vim. I use emacs for coding, and nano editor for editing simple text files, and input commit log. I invoke aspell from nano (Ctrl-t, Ctrl-s), but I sometimes forget to do spell checking. > > > > config DEBUG_INFO_DWARF5 > > bool "Generate DWARF Version 5 debuginfo" > > - depends on GCC_VERSION >= 50000 || CC_IS_CLANG > > - depends on CC_IS_GCC || $(success,$(srctree)/scripts/test_dwarf5_support.sh $(CC) $(CLANG_FLAGS)) > > + depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && AS_IS_GNU && AS_VERSION >= 23502) || (CC_IS_CLANG && AS_IS_LLVM) > > Would this be more concise as: > + depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && (AS_IS_LLVM > || (AS_IS_GNU && AS_VERSION >= 23502)) > Yes, this is simpler. I will do it in v2. Thanks. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] kbuild: dwarf: use AS_VERSION instead of test_dwarf5_support.sh 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-04 0:30 ` Nathan Chancellor 1 sibling, 0 replies; 20+ messages in thread From: Nathan Chancellor @ 2021-03-04 0:30 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, clang-built-linux, Nick Desaulniers, linux-kernel On Thu, Mar 04, 2021 at 03:33:33AM +0900, Masahiro Yamada wrote: > The test code in scripts/test_dwarf5_support.sh is somewhat difficult > to understand, but after all, we want to check binutils >= 2.35.2 > > >From the former discussion, the requrement for generating DRAWF v5 from > C code is as follows: > > - gcc + binutils as -> requires gcc 5.0+ (but 7.0+ for full support) > - clang + binutils as -> requires binutils 2.35.2+ > - clang + integrated as -> OK > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > > lib/Kconfig.debug | 3 +-- > scripts/test_dwarf5_support.sh | 8 -------- > 2 files changed, 1 insertion(+), 10 deletions(-) > delete mode 100755 scripts/test_dwarf5_support.sh > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 2779c29d9981..f3337a38925d 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -284,8 +284,7 @@ config DEBUG_INFO_DWARF4 > > config DEBUG_INFO_DWARF5 > bool "Generate DWARF Version 5 debuginfo" > - depends on GCC_VERSION >= 50000 || CC_IS_CLANG > - depends on CC_IS_GCC || $(success,$(srctree)/scripts/test_dwarf5_support.sh $(CC) $(CLANG_FLAGS)) > + depends on GCC_VERSION >= 50000 || (CC_IS_CLANG && AS_IS_GNU && AS_VERSION >= 23502) || (CC_IS_CLANG && AS_IS_LLVM) > depends on !DEBUG_INFO_BTF > help > Generate DWARF v5 debug info. Requires binutils 2.35.2, gcc 5.0+ (gcc > diff --git a/scripts/test_dwarf5_support.sh b/scripts/test_dwarf5_support.sh > deleted file mode 100755 > index c46e2456b47a..000000000000 > --- a/scripts/test_dwarf5_support.sh > +++ /dev/null > @@ -1,8 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > - > -# Test that the assembler doesn't need -Wa,-gdwarf-5 when presented with DWARF > -# v5 input, such as `.file 0` and `md5 0x00`. Should be fixed in GNU binutils > -# 2.35.2. https://sourceware.org/bugzilla/show_bug.cgi?id=25611 > -echo '.file 0 "filename" md5 0x7a0b65214090b6693bd1dc24dd248245' | \ > - $* -gdwarf-5 -Wno-unused-command-line-argument -c -x assembler -o /dev/null - > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG 2021-03-03 18:33 [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Masahiro Yamada ` (2 preceding siblings ...) 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:47 ` Nick Desaulniers 2021-03-05 6:06 ` Masahiro Yamada 2021-03-08 19:11 ` Sami Tolvanen 2021-03-03 23:37 ` Nathan Chancellor 4 siblings, 2 replies; 20+ messages in thread From: Nick Desaulniers @ 2021-03-03 20:47 UTC (permalink / raw) To: Masahiro Yamada, Sami Tolvanen Cc: Linux Kbuild mailing list, clang-built-linux, Nathan Chancellor, LKML + 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG 2021-03-03 20:47 ` [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Nick Desaulniers @ 2021-03-05 6:06 ` Masahiro Yamada 2021-03-08 19:11 ` Sami Tolvanen 1 sibling, 0 replies; 20+ messages in thread From: Masahiro Yamada @ 2021-03-05 6:06 UTC (permalink / raw) To: Nick Desaulniers Cc: Sami Tolvanen, Linux Kbuild mailing list, clang-built-linux, Nathan Chancellor, LKML On Thu, Mar 4, 2021 at 5:47 AM 'Nick Desaulniers' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > + 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. 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. > > > > 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. > > > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG 2021-03-03 20:47 ` [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Nick Desaulniers 2021-03-05 6:06 ` Masahiro Yamada @ 2021-03-08 19:11 ` Sami Tolvanen 1 sibling, 0 replies; 20+ messages in thread From: Sami Tolvanen @ 2021-03-08 19:11 UTC (permalink / raw) To: Nick Desaulniers Cc: Masahiro Yamada, Linux Kbuild mailing list, clang-built-linux, Nathan Chancellor, LKML On Wed, Mar 3, 2021 at 12:47 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > + 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. I'm fine with removing the check, but the idea here was to just make it slightly harder for people to accidentally use a mismatched toolchain, even though checking for LLVM=1 doesn't stop them from doing so anyway. But yes, the only LLVM tools required in addition to the compiler and the linker are llvm-ar and llvm-nm. Sami ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG 2021-03-03 18:33 [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Masahiro Yamada ` (3 preceding siblings ...) 2021-03-03 20:47 ` [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Nick Desaulniers @ 2021-03-03 23:37 ` Nathan Chancellor 4 siblings, 0 replies; 20+ messages in thread From: Nathan Chancellor @ 2021-03-03 23:37 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, clang-built-linux, Nick Desaulniers, linux-kernel On Thu, Mar 04, 2021 at 03:33:30AM +0900, Masahiro Yamada 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. > > 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> Yes, there is not too much point of checking if $(LLVM) is set or not because we already check for the other tools. Reviewed-by: Nathan Chancellor <nathan@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) > 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 > ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-03-12 17:23 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 ` [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG Nick Desaulniers 2021-03-05 6:06 ` Masahiro Yamada 2021-03-08 19:11 ` Sami Tolvanen 2021-03-03 23:37 ` Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).