Add version macros for clang and fix arm64 for clang <6.0

PATCH Superseded Sami Tolvanen 4 participants 15 messages Nov 29, 2017 – Nov 30, 2017

Subsystems (2)

Kbuild arm64

lore

Share & tools

lore

Sami Tolvanen

author

2017-11-29 00:00 UTC #0 lore raw

cover Add version macros for clang and fix arm64 for clang <6.0

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

Signature · 1 line

2.15.0.417.g466bffb3ac-goog

Sami Tolvanen

author

2017-11-29 00:00 UTC #1 lore raw

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

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

Signed-off-by Sami Tolvanen <[email protected]>

---
 scripts/clang-version.sh | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100755 scripts/clang-version.sh

scripts/clang-version.sh new file +33 -0

new file mode 100755
index 000000000000..5f14ddc54032
@@ -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

Signature · 1 line

2.15.0.417.g466bffb3ac-goog

Nick Desaulniers

Tested-by on v1 1/3

2017-11-29 17:39 UTC #2 lore raw

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

+# 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 <[email protected]>

Masahiro Yamada

2017-11-30 12:09 UTC #3 lore raw

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

2017-11-30 2:39 GMT+09:00 Nick Desaulniers <[email protected]>:

Quoted 6 lines

scripts/clang-version.sh new file +3 -0
+# 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.

Quoted 9 lines

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 <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Best Regards
Masahiro Yamada

Sami Tolvanen

author

2017-11-29 00:00 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 scripts/Kbuild.include | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

scripts/Kbuild.include +31 -0

index 065324a8046f..b6d7d347b203 100644
@@ -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))

Signature · 1 line

2.15.0.417.g466bffb3ac-goog

Nick Desaulniers

2017-11-29 17:19 UTC #5 lore raw

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

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

Masahiro Yamada

2017-11-30 12:21 UTC #6 lore raw

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

In reply to Sami Tolvanen

2017-11-29 9:00 GMT+09:00 Sami Tolvanen <[email protected]>:

Quoted 21 lines

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 <[email protected]>
---
 scripts/Kbuild.include | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
scripts/Kbuild.include +3 -0
index 065324a8046f..b6d7d347b203 100644
@@ -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))

scripts/Kbuild.include cc-disable-warning = $(call try-run-cached,\ · line 230

Quoted 10 lines

scripts/Kbuild.include +10 -0

+
+# __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.

scripts/Kbuild.include cc-disable-warning = $(call try-run-cached,\ · line 235

Quoted 4 lines

scripts/Kbuild.include +4 -0

+# 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?

scripts/Kbuild.include cc-disable-warning = $(call try-run-cached,\ · line 248

Quoted 12 lines

scripts/Kbuild.include +12 -0

+# 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)

Quoted 10 lines

scripts/Kbuild.include +0 -1
 # 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 at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Best Regards
Masahiro Yamada

Sami Tolvanen

author

2017-11-30 17:34 UTC #7 lore raw

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

Quoted 1 line · Masahiro Yamada

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.

Quoted 9 lines

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

Sami Tolvanen

author

2017-11-29 00:00 UTC #8 lore raw

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

In reply to Sami Tolvanen

From: Greg Hackmann <[email protected]>

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 <[email protected]>

Cc Matthias Kaehlcke <[email protected]>

[added clang-ifversion to enable the workaround only for clang <6.0]

Signed-off-by Sami Tolvanen <[email protected]>

---
 arch/arm64/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/Makefile +4 -1

index b35788c909f1..d2efb5a0212f 100644
@@ -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)

Signature · 1 line

2.15.0.417.g466bffb3ac-goog

Ard Biesheuvel

2017-11-29 12:15 UTC #9 lore raw

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

Quoted 6 lines · Sami Tolvanen

From: Greg Hackmann <[email protected]>
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?

Quoted 27 lines

Signed-off-by Greg Hackmann <[email protected]>

Cc Matthias Kaehlcke <[email protected]>

[added clang-ifversion to enable the workaround only for clang <6.0]

Signed-off-by Sami Tolvanen <[email protected]>

---
 arch/arm64/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/Makefile +4 -2

index b35788c909f1..d2efb5a0212f 100644
@@ -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

Sami Tolvanen

author

2017-11-29 16:22 UTC #10 lore raw

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

Quoted 1 line · Ard Biesheuvel

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

Nick Desaulniers

2017-11-29 17:26 UTC #11 lore raw

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

Quoted 6 lines · Sami Tolvanen

Quoted 1 line · Ard Biesheuvel
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?

Sami Tolvanen

author

2017-11-30 23:38 UTC #12 lore raw

v2 cover Add version macros for clang

In reply to 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

Signature · 1 line

2.15.0.531.g2ccb3012c9-goog

Sami Tolvanen

author

2017-11-30 23:38 UTC #13 lore raw

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

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

Signed-off-by Sami Tolvanen <[email protected]>

Tested-by Nick Desaulniers <[email protected]>

---
 scripts/clang-version.sh | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100755 scripts/clang-version.sh

scripts/clang-version.sh new file +33 -0

new file mode 100755
index 000000000000..9780efa56980
@@ -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

Signature · 1 line

2.15.0.531.g2ccb3012c9-goog

Sami Tolvanen

author

2017-11-30 23:38 UTC #14 lore raw

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

In reply to Sami Tolvanen

Signed-off-by Sami Tolvanen <[email protected]>

---
 scripts/Kbuild.include | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

scripts/Kbuild.include +34 -7

index 065324a8046f..eb2cf2780f6e 100644
@@ -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)

Signature · 1 line

2.15.0.531.g2ccb3012c9-goog