[PATCH 0/3] Add version macros for clang and fix arm64 for clang <6.0

[PATCH 0/3] Add version macros for clang and fix arm64 for clang <6.0

[Sami Tolvanen]

In order to build an arm64 kernel with clang <6.0, we need to work
around LLVM bug 30792 by using -mno-implicit-float instead of
-mgeneral-regs-only.

This patch set adds macros for checking clang version, and enable the
workaround only for affected compiler versions.

Greg Hackmann (1):
  arm64: use -mno-implicit-float instead of -mgeneral-regs-only

Sami Tolvanen (2):
  kbuild: add clang-version.sh
  kbuild: add cc-if-name-version and compiler-specific variants

 arch/arm64/Makefile      |  5 ++++-
 scripts/Kbuild.include   | 31 +++++++++++++++++++++++++++++++
 scripts/clang-version.sh | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100755 scripts/clang-version.sh

-- 
2.15.0.417.g466bffb3ac-goog

[PATCH 1/3] kbuild: add clang-version.sh

[Sami Tolvanen]

Based on gcc-version.sh, clang-version.sh prints out the correct
version of clang.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/clang-version.sh | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100755 scripts/clang-version.sh

diff --git a/scripts/clang-version.sh b/scripts/clang-version.sh
new file mode 100755
index 000000000000..5f14ddc54032
--- /dev/null
+++ b/scripts/clang-version.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# clang-version [-p] clang-command
+#
+# Prints the compiler version of `command' in a canonical 4-digit form
+# such as `0500' for clang-5.0 etc.
+#
+# With the -p option, prints the patchlevel as well, for example `050001' for
+# clang-5.0.1 etc.
+#
+
+if [ "$1" = "-p" ] ; then
+	with_patchlevel=1;
+	shift;
+fi
+
+compiler="$*"
+
+if [ ${#compiler} -eq 0 ]; then
+	echo "Error: No compiler specified."
+	printf "Usage:\n\t$0 <clang-command>\n"
+	exit 1
+fi
+
+MAJOR=$(echo __clang_major__ | $compiler -E -x c - | tail -n 1)
+MINOR=$(echo __clang_minor__ | $compiler -E -x c - | tail -n 1)
+if [ "x$with_patchlevel" != "x" ] ; then
+	PATCHLEVEL=$(echo __clang_patchlevel__ | $compiler -E -x c - | tail -n 1)
+	printf "%02d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL
+else
+	printf "%02d%02d\\n" $MAJOR $MINOR
+fi
-- 
2.15.0.417.g466bffb3ac-goog

Re: [PATCH 1/3] kbuild: add clang-version.sh

[Nick Desaulniers]

+# clang-version [-p] clang-command
+#
+# Prints the compiler version of `command' in a canonical 4-digit form

small nit: `command` should be `clang-command`, but its just a comment
(maybe the maintainer can make that change when/if applying).

The conditional at the end to see if $with_patchlevel using "x" is
odd, but this is what scripts/gcc-version.sh does and this patch does
work as expected.

Tested-by: Nick Desaulniers <ndesaulniers@google.com>

Re: [PATCH 1/3] kbuild: add clang-version.sh

[Masahiro Yamada]

2017-11-30 2:39 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>:
> +# clang-version [-p] clang-command
> +#
> +# Prints the compiler version of `command' in a canonical 4-digit form
>
> small nit: `command` should be `clang-command`, but its just a comment
> (maybe the maintainer can make that change when/if applying).


I would be happy to fix it up locally if there were no other issue.


> The conditional at the end to see if $with_patchlevel using "x" is
> odd, but this is what scripts/gcc-version.sh does and this patch does
> work as expected.
>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

[PATCH 2/3] kbuild: add cc-if-name-version and compiler-specific variants

[Sami Tolvanen]

This change adds macros for testing both compiler name and
version. Current cc-version, cc-ifversion etc. macros that test
gcc version are left unchanged to prevent compatibility issues
with existing tests.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/Kbuild.include | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 065324a8046f..b6d7d347b203 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -215,6 +215,37 @@ cc-disable-warning = $(call try-run-cached,\
 # Expands to either gcc or clang
 cc-name = $(call shell-cached,$(CC) -v 2>&1 | grep -q "clang version" && echo clang || echo gcc)
 
+# __cc-version
+# Returns compiler version
+__cc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/$(cc-name)-version.sh $(CC))
+
+# __cc-fullversion
+# Returns full compiler version
+__cc-fullversion = $(shell $(CONFIG_SHELL) \
+	$(srctree)/scripts/$(cc-name)-version.sh -p $(CC))
+
+# cc-if-name-version
+# Matches compiler name and version
+# Usage:  EXTRA_CFLAGS += $(call cc-if-name-version, gcc, -lt, 0402, -O1)
+cc-if-name-version = $(shell [ $(cc-name) == $(1) ] && [ $(__cc-version) $(2) $(3) ] && echo $(4) || echo $(5))
+
+# cc-if-name-fullversion
+# Matches compiler name and full version
+# Usage:  EXTRA_CFLAGS += $(call cc-if-name-fullversion, gcc, -lt, 040502, -O1)
+cc-if-name-fullversion = $(shell [ $(cc-name) == $(1) ] && [ $(__cc-fullversion) $(2) $(3) ] && echo $(4) || echo $(5))
+
+# gcc-ifversion
+gcc-ifversion = $(call cc-if-name-version, gcc, $(1), $(2), $(3), $(4))
+
+# gcc-if-fullversion
+gcc-if-fullversion = (call cc-if-name-fullversion, gcc, $(1), $(2), $(3), $(4))
+
+# clang-ifversion
+clang-ifversion =  $(call cc-if-name-version, clang, $(1), $(2), $(3), $(4))
+
+# clang-if-fullversion
+clang-if-fullversion = (call cc-if-name-fullversion, clang, $(1), $(2), $(3), $(4))
+
 # cc-version
 cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC))
 
-- 
2.15.0.417.g466bffb3ac-goog

Re: [PATCH 2/3] kbuild: add cc-if-name-version and compiler-specific variants

[Nick Desaulniers]

We could use something like this to warn people trying to build the
kernel with clang-3.8 for instance.

Re: [PATCH 2/3] kbuild: add cc-if-name-version and compiler-specific variants

[Masahiro Yamada]

2017-11-29 9:00 GMT+09:00 Sami Tolvanen <samitolvanen@google.com>:
> This change adds macros for testing both compiler name and
> version. Current cc-version, cc-ifversion etc. macros that test
> gcc version are left unchanged to prevent compatibility issues
> with existing tests.
>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
>  scripts/Kbuild.include | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 065324a8046f..b6d7d347b203 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -215,6 +215,37 @@ cc-disable-warning = $(call try-run-cached,\
>  # Expands to either gcc or clang
>  cc-name = $(call shell-cached,$(CC) -v 2>&1 | grep -q "clang version" && echo clang || echo gcc)
>
> +# __cc-version
> +# Returns compiler version
> +__cc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/$(cc-name)-version.sh $(CC))
> +
> +# __cc-fullversion
> +# Returns full compiler version
> +__cc-fullversion = $(shell $(CONFIG_SHELL) \
> +       $(srctree)/scripts/$(cc-name)-version.sh -p $(CC))
> +
> +# cc-if-name-version
> +# Matches compiler name and version
> +# Usage:  EXTRA_CFLAGS += $(call cc-if-name-version, gcc, -lt, 0402, -O1)
> +cc-if-name-version = $(shell [ $(cc-name) == $(1) ] && [ $(__cc-version) $(2) $(3) ] && echo $(4) || echo $(5))


BTW, did this patch work in your test?

After applying this series, I saw "/bin/sh: 1: [: gcc: unexpected operator"


$ make ARCH=arm64  CROSS_COMPILE=aarch64-linux-gnu-   defconfig
/bin/sh: 1: [: gcc: unexpected operator
*** Default configuration is based on 'defconfig'
#
# configuration written to .config
#


I needed to change

[ $(cc-name) == $(1) ]

into

[ $(cc-name) = $(1) ]

to make it work.



> +# cc-if-name-fullversion
> +# Matches compiler name and full version
> +# Usage:  EXTRA_CFLAGS += $(call cc-if-name-fullversion, gcc, -lt, 040502, -O1)
> +cc-if-name-fullversion = $(shell [ $(cc-name) == $(1) ] && [ $(__cc-fullversion) $(2) $(3) ] && echo $(4) || echo $(5))

Ditto.

I fixed '==' into '=' to make this work as expected.




As for the macro names, maybe

__cc-ifversion, __cc-if-fullversion for consistency?




> +# gcc-ifversion
> +gcc-ifversion = $(call cc-if-name-version, gcc, $(1), $(2), $(3), $(4))
> +
> +# gcc-if-fullversion
> +gcc-if-fullversion = (call cc-if-name-fullversion, gcc, $(1), $(2), $(3), $(4))
> +
> +# clang-ifversion
> +clang-ifversion =  $(call cc-if-name-version, clang, $(1), $(2), $(3), $(4))
> +
> +# clang-if-fullversion
> +clang-if-fullversion = (call cc-if-name-fullversion, clang, $(1), $(2), $(3), $(4))
> +

I think you can do like follows to avoid code duplication.

# backward compatibility
cc-ifversion = $(gcc-ifversion)
cc-if-fullversion = $(gcc-if-fullversion)




>  # cc-version
>  cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC))
>
> --
> 2.15.0.417.g466bffb3ac-goog
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] kbuild: add cc-if-name-version and compiler-specific variants

[Sami Tolvanen]

On Thu, Nov 30, 2017 at 09:21:49PM +0900, Masahiro Yamada wrote:
> BTW, did this patch work in your test?

Yes, but it looks like /bin/sh is symlinked to bash on my system. I can
reproduce the issue with dash.

> As for the macro names, maybe
> 
> __cc-ifversion, __cc-if-fullversion for consistency?

> I think you can do like follows to avoid code duplication.
> 
> # backward compatibility
> cc-ifversion = $(gcc-ifversion)
> cc-if-fullversion = $(gcc-if-fullversion)

Sure, both changes sound good to me. I'll fix these in v2.

Sami

[PATCH 3/3] arm64: use -mno-implicit-float instead of -mgeneral-regs-only

[Sami Tolvanen]

From: Greg Hackmann <ghackmann@google.com>

LLVM bug 30792 causes clang's AArch64 backend to crash compiling
arch/arm64/crypto/aes-ce-cipher.c.  Replacing -mgeneral-regs-only with
-mno-implicit-float is the suggested workaround.

Signed-off-by: Greg Hackmann <ghackmann@google.com>
Cc: Matthias Kaehlcke <mka@chromium.org>
[added clang-ifversion to enable the workaround only for clang <6.0]
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 arch/arm64/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index b35788c909f1..d2efb5a0212f 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -49,7 +49,10 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
   endif
 endif
 
-KBUILD_CFLAGS	+= -mgeneral-regs-only $(lseinstr) $(brokengasinst)
+# This is a workaround for https://bugs.llvm.org/show_bug.cgi?id=30792.
+KBUILD_CFLAGS	+= $(call clang-ifversion, -lt, 0600, -mno-implicit-float, -mgeneral-regs-only)
+
+KBUILD_CFLAGS	+= $(lseinstr) $(brokengasinst)
 KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables
 KBUILD_CFLAGS	+= $(call cc-option, -mpc-relative-literal-loads)
 KBUILD_AFLAGS	+= $(lseinstr) $(brokengasinst)
-- 
2.15.0.417.g466bffb3ac-goog

Re: [PATCH 3/3] arm64: use -mno-implicit-float instead of -mgeneral-regs-only

[Ard Biesheuvel]

On 29 November 2017 at 00:00, Sami Tolvanen <samitolvanen@google.com> wrote:
> From: Greg Hackmann <ghackmann@google.com>
>
> LLVM bug 30792 causes clang's AArch64 backend to crash compiling
> arch/arm64/crypto/aes-ce-cipher.c.  Replacing -mgeneral-regs-only with
> -mno-implicit-float is the suggested workaround.
>

Do we still need these patches now that the AES code has been fixed?

> Signed-off-by: Greg Hackmann <ghackmann@google.com>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> [added clang-ifversion to enable the workaround only for clang <6.0]
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
>  arch/arm64/Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index b35788c909f1..d2efb5a0212f 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -49,7 +49,10 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
>    endif
>  endif
>
> -KBUILD_CFLAGS  += -mgeneral-regs-only $(lseinstr) $(brokengasinst)
> +# This is a workaround for https://bugs.llvm.org/show_bug.cgi?id=30792.
> +KBUILD_CFLAGS  += $(call clang-ifversion, -lt, 0600, -mno-implicit-float, -mgeneral-regs-only)
> +
> +KBUILD_CFLAGS  += $(lseinstr) $(brokengasinst)
>  KBUILD_CFLAGS  += -fno-asynchronous-unwind-tables
>  KBUILD_CFLAGS  += $(call cc-option, -mpc-relative-literal-loads)
>  KBUILD_AFLAGS  += $(lseinstr) $(brokengasinst)
> --
> 2.15.0.417.g466bffb3ac-goog
>

Re: [PATCH 3/3] arm64: use -mno-implicit-float instead of -mgeneral-regs-only

[Sami Tolvanen]

On Wed, Nov 29, 2017 at 12:15:14PM +0000, Ard Biesheuvel wrote:
> Do we still need these patches now that the AES code has been fixed?

With your AES patch that Herbert just applied, this patch is no longer
needed. Version macros in the first two patches will still be useful in
future though.

Sami

Re: [PATCH 3/3] arm64: use -mno-implicit-float instead of -mgeneral-regs-only

[Nick Desaulniers]

On Wed, Nov 29, 2017 at 8:22 AM, Sami Tolvanen <samitolvanen@google.com> wrote:
> On Wed, Nov 29, 2017 at 12:15:14PM +0000, Ard Biesheuvel wrote:
>> Do we still need these patches now that the AES code has been fixed?
>
> With your AES patch that Herbert just applied, this patch is no longer
> needed. Version macros in the first two patches will still be useful in
> future though.

Matthias, maybe for your recommended Clang patch sets, you can replace
this patch with
https://www.spinics.net/lists/arm-kernel/msg617947.html?

[PATCH v2 0/2] Add version macros for clang

[Sami Tolvanen]

This patch set adds macros for checking clang version.

Changes from v1:
  - fixed a comment in clang-version.sh
  - renamed macros to __cc-ifversion and __cc-if-fullversion
  - fixed a bug with non-bash shells
  - dropped the arm64 patch as unnecessary

Sami Tolvanen (2):
  kbuild: add clang-version.sh
  kbuild: add __cc-ifversion and compiler-specific variants

 scripts/Kbuild.include   | 41 ++++++++++++++++++++++++++++++++++-------
 scripts/clang-version.sh | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 7 deletions(-)
 create mode 100755 scripts/clang-version.sh

-- 
2.15.0.531.g2ccb3012c9-goog

[PATCH v2 1/2] kbuild: add clang-version.sh

[Sami Tolvanen]

Based on gcc-version.sh, clang-version.sh prints out the correct
version of clang.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
---
 scripts/clang-version.sh | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100755 scripts/clang-version.sh

diff --git a/scripts/clang-version.sh b/scripts/clang-version.sh
new file mode 100755
index 000000000000..9780efa56980
--- /dev/null
+++ b/scripts/clang-version.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# clang-version [-p] clang-command
+#
+# Prints the compiler version of `clang-command' in a canonical 4-digit form
+# such as `0500' for clang-5.0 etc.
+#
+# With the -p option, prints the patchlevel as well, for example `050001' for
+# clang-5.0.1 etc.
+#
+
+if [ "$1" = "-p" ] ; then
+	with_patchlevel=1;
+	shift;
+fi
+
+compiler="$*"
+
+if [ ${#compiler} -eq 0 ]; then
+	echo "Error: No compiler specified."
+	printf "Usage:\n\t$0 <clang-command>\n"
+	exit 1
+fi
+
+MAJOR=$(echo __clang_major__ | $compiler -E -x c - | tail -n 1)
+MINOR=$(echo __clang_minor__ | $compiler -E -x c - | tail -n 1)
+if [ "x$with_patchlevel" != "x" ] ; then
+	PATCHLEVEL=$(echo __clang_patchlevel__ | $compiler -E -x c - | tail -n 1)
+	printf "%02d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL
+else
+	printf "%02d%02d\\n" $MAJOR $MINOR
+fi
-- 
2.15.0.531.g2ccb3012c9-goog

[PATCH v2 2/2] kbuild: add __cc-ifversion and compiler-specific variants

[Sami Tolvanen]

This change adds macros for testing both compiler name and
version. Current cc-version, cc-ifversion etc. macros that test
gcc version are left unchanged to prevent compatibility issues
with existing tests.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/Kbuild.include | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 065324a8046f..eb2cf2780f6e 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -215,6 +215,37 @@ cc-disable-warning = $(call try-run-cached,\
 # Expands to either gcc or clang
 cc-name = $(call shell-cached,$(CC) -v 2>&1 | grep -q "clang version" && echo clang || echo gcc)
 
+# __cc-version
+# Returns compiler version
+__cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/$(cc-name)-version.sh $(CC))
+
+# __cc-fullversion
+# Returns full compiler version
+__cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
+	$(srctree)/scripts/$(cc-name)-version.sh -p $(CC))
+
+# __cc-ifversion
+# Matches compiler name and version
+# Usage:  EXTRA_CFLAGS += $(call cc-if-name-version, gcc, -lt, 0402, -O1)
+__cc-ifversion = $(shell [ $(cc-name) = $(1) ] && [ $(__cc-version) $(2) $(3) ] && echo $(4) || echo $(5))
+
+# __cc-if-fullversion
+# Matches compiler name and full version
+# Usage:  EXTRA_CFLAGS += $(call cc-if-name-fullversion, gcc, -lt, 040502, -O1)
+__cc-if-fullversion = $(shell [ $(cc-name) = $(1) ] && [ $(__cc-fullversion) $(2) $(3) ] && echo $(4) || echo $(5))
+
+# gcc-ifversion
+gcc-ifversion = $(call __cc-ifversion, gcc, $(1), $(2), $(3), $(4))
+
+# gcc-if-fullversion
+gcc-if-fullversion = (call __cc-if-fullversion, gcc, $(1), $(2), $(3), $(4))
+
+# clang-ifversion
+clang-ifversion =  $(call __cc-ifversion, clang, $(1), $(2), $(3), $(4))
+
+# clang-if-fullversion
+clang-if-fullversion = (call __cc-if-fullversion, clang, $(1), $(2), $(3), $(4))
+
 # cc-version
 cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.sh $(CC))
 
@@ -222,13 +253,9 @@ cc-version = $(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/gcc-version.
 cc-fullversion = $(call shell-cached,$(CONFIG_SHELL) \
 	$(srctree)/scripts/gcc-version.sh -p $(CC))
 
-# cc-ifversion
-# Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
-cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
-
-# cc-if-fullversion
-# Usage:  EXTRA_CFLAGS += $(call cc-if-fullversion, -lt, 040502, -O1)
-cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo $(4))
+# backward compatibility
+cc-ifversion = $(gcc-ifversion)
+cc-if-fullversion = $(gcc-if-fullversion)
 
 # cc-ldoption
 # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
-- 
2.15.0.531.g2ccb3012c9-goog