* [PATCH] arm64: crypto: Disable xor-neon build when using clang
@ 2019-02-12 19:46 Mark Brown
2019-02-12 20:33 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2019-02-12 19:46 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Ard Biesheuvel, Kevin Hilman, Jackie Liu, ndesaulniers,
Mark Brown, Nathan Chancellor, linux-arm-kernel
Currently clang is able to build an arm64 defconfig or allmodconfig
using the release branch for clang 8 apart from a series of errors in
xor-neon.c in the form:
arch/arm64/lib/xor-neon.c:27:28: error: incompatible pointer types assigning to 'const unsigned long *' from 'uint64_t *' (aka 'unsigned long long *') [-Werror,-Wincompatible-pointer-types]
This issue has been extensively discussed between various interested
parties but it is still unclear what if any the best solution is in the
compiler and there are concerns about just disabling the warning with a
pragma in the header due to the potential for masking other serious
issues. Details of the discussion can be followed here:
https://github.com/ClangBuiltLinux/linux/issues/283
In order to avoid the issue and facilitate further clang work and
testing for the time being just disable the use of xor-neon.c if we are
building with clang. Obviously this is not actually resolving the
problem and is not something that should be with us for the long term
but since it only affects clang and enables people working on that to
work directly with upstream it seems like a useful tradeoff.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
It would also be possible to achieve the same effect by using either a
Makefile change or a pragma to disable the warning for this specific
file, I can send a patch for those approaches if that's OK and
preferable. Disabling the acceleration entirely seemed safer and less
likely to be forgotten to me.
We've got some KernelCI support for clang in the middle of getting
deployed:
https://staging.kernelci.org/job/next-clang/branch/master/
so being able to build and run upstream kernels directly would be
enormously helpful.
arch/arm64/include/asm/xor.h | 2 +-
arch/arm64/lib/Makefile | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/xor.h b/arch/arm64/include/asm/xor.h
index 856386ad076c..971b8336d2d2 100644
--- a/arch/arm64/include/asm/xor.h
+++ b/arch/arm64/include/asm/xor.h
@@ -14,7 +14,7 @@
#include <asm/hwcap.h>
#include <asm/neon.h>
-#ifdef CONFIG_KERNEL_MODE_NEON
+#if defined(CONFIG_KERNEL_MODE_NEON) && !defined(CONFIG_CC_IS_CLANG)
extern struct xor_block_template const xor_block_inner_neon;
diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index 5540a1638baf..b2a5b8d56354 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -6,10 +6,12 @@ lib-y := clear_user.o delay.o copy_from_user.o \
strchr.o strrchr.o tishift.o
ifeq ($(CONFIG_KERNEL_MODE_NEON), y)
+ifneq ($(CONFIG_CC_IS_CLANG), y)
obj-$(CONFIG_XOR_BLOCKS) += xor-neon.o
CFLAGS_REMOVE_xor-neon.o += -mgeneral-regs-only
CFLAGS_xor-neon.o += -ffreestanding
endif
+endif
# Tell the compiler to treat all general purpose registers (with the
# exception of the IP registers, which are already handled by the caller
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: crypto: Disable xor-neon build when using clang
2019-02-12 19:46 [PATCH] arm64: crypto: Disable xor-neon build when using clang Mark Brown
@ 2019-02-12 20:33 ` Ard Biesheuvel
2019-02-13 11:40 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2019-02-12 20:33 UTC (permalink / raw)
To: Mark Brown
Cc: Catalin Marinas, Jackie Liu, Nick Desaulniers, Will Deacon,
Kevin Hilman, Nathan Chancellor, linux-arm-kernel
On Tue, 12 Feb 2019 at 20:46, Mark Brown <broonie@kernel.org> wrote:
>
> Currently clang is able to build an arm64 defconfig or allmodconfig
> using the release branch for clang 8 apart from a series of errors in
> xor-neon.c in the form:
>
> arch/arm64/lib/xor-neon.c:27:28: error: incompatible pointer types assigning to 'const unsigned long *' from 'uint64_t *' (aka 'unsigned long long *') [-Werror,-Wincompatible-pointer-types]
>
> This issue has been extensively discussed between various interested
> parties but it is still unclear what if any the best solution is in the
> compiler and there are concerns about just disabling the warning with a
> pragma in the header due to the potential for masking other serious
> issues. Details of the discussion can be followed here:
>
> https://github.com/ClangBuiltLinux/linux/issues/283
>
> In order to avoid the issue and facilitate further clang work and
> testing for the time being just disable the use of xor-neon.c if we are
> building with clang. Obviously this is not actually resolving the
> problem and is not something that should be with us for the long term
> but since it only affects clang and enables people working on that to
> work directly with upstream it seems like a useful tradeoff.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>
> It would also be possible to achieve the same effect by using either a
> Makefile change or a pragma to disable the warning for this specific
> file, I can send a patch for those approaches if that's OK and
> preferable. Disabling the acceleration entirely seemed safer and less
> likely to be forgotten to me.
>
> We've got some KernelCI support for clang in the middle of getting
> deployed:
>
> https://staging.kernelci.org/job/next-clang/branch/master/
>
> so being able to build and run upstream kernels directly would be
> enormously helpful.
>
> arch/arm64/include/asm/xor.h | 2 +-
> arch/arm64/lib/Makefile | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/xor.h b/arch/arm64/include/asm/xor.h
> index 856386ad076c..971b8336d2d2 100644
> --- a/arch/arm64/include/asm/xor.h
> +++ b/arch/arm64/include/asm/xor.h
> @@ -14,7 +14,7 @@
> #include <asm/hwcap.h>
> #include <asm/neon.h>
>
> -#ifdef CONFIG_KERNEL_MODE_NEON
> +#if defined(CONFIG_KERNEL_MODE_NEON) && !defined(CONFIG_CC_IS_CLANG)
>
> extern struct xor_block_template const xor_block_inner_neon;
>
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index 5540a1638baf..b2a5b8d56354 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -6,10 +6,12 @@ lib-y := clear_user.o delay.o copy_from_user.o \
> strchr.o strrchr.o tishift.o
>
> ifeq ($(CONFIG_KERNEL_MODE_NEON), y)
> +ifneq ($(CONFIG_CC_IS_CLANG), y)
> obj-$(CONFIG_XOR_BLOCKS) += xor-neon.o
> CFLAGS_REMOVE_xor-neon.o += -mgeneral-regs-only
> CFLAGS_xor-neon.o += -ffreestanding
> endif
> +endif
>
> # Tell the compiler to treat all general purpose registers (with the
> # exception of the IP registers, which are already handled by the caller
> --
> 2.20.1
>
Can we just add the pragma to neon-instrinsics.h and be done with it?
As long as Clang is not the dominant compiler, I don't buy the 'hiding
real bugs' argument, especially since neon-intrinsics.h should never
be included transitively, so it only affects source files that
actually call NEON intrinsics.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: crypto: Disable xor-neon build when using clang
2019-02-12 20:33 ` Ard Biesheuvel
@ 2019-02-13 11:40 ` Mark Brown
2019-02-13 21:45 ` Nick Desaulniers
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2019-02-13 11:40 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Catalin Marinas, Jackie Liu, Nick Desaulniers, Will Deacon,
Kevin Hilman, Nathan Chancellor, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 507 bytes --]
On Tue, Feb 12, 2019 at 08:33:57PM +0000, Ard Biesheuvel wrote:
> Can we just add the pragma to neon-instrinsics.h and be done with it?
> As long as Clang is not the dominant compiler, I don't buy the 'hiding
> real bugs' argument, especially since neon-intrinsics.h should never
> be included transitively, so it only affects source files that
> actually call NEON intrinsics.
That's fine from my point of view, when posting this I was basically
just going for the most conservative workaround possible.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: crypto: Disable xor-neon build when using clang
2019-02-13 11:40 ` Mark Brown
@ 2019-02-13 21:45 ` Nick Desaulniers
2019-02-14 16:30 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2019-02-13 21:45 UTC (permalink / raw)
To: Mark Brown, Ard Biesheuvel
Cc: Catalin Marinas, Jackie Liu, Will Deacon, Stephen Hines,
Kevin Hilman, Nathan Chancellor, linux-arm-kernel
On Wed, Feb 13, 2019 at 3:40 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Feb 12, 2019 at 08:33:57PM +0000, Ard Biesheuvel wrote:
>
> > Can we just add the pragma to neon-instrinsics.h and be done with it?
> > As long as Clang is not the dominant compiler, I don't buy the 'hiding
> > real bugs' argument, especially since neon-intrinsics.h should never
> > be included transitively, so it only affects source files that
> > actually call NEON intrinsics.
>
> That's fine from my point of view, when posting this I was basically
> just going for the most conservative workaround possible.
Idealistically, I'm not really happy with either. But pragmatically,
if this is the last thing preventing us from turning on KernelCI
support for arm64+clang; I'll pick the lesser of two evils, which
would be the pragma (vs disabling the whole translation unit). We can
always revisit why all of this complexity is going into this
particular part of the code later when we have more time. I'm focused
right now on finding bugs in llvm asm goto support so that it works
well when it lands, and upstreaming lld support, but hopefully we can
revisit this more later, when I have more time and energy.
Nathan's patch [0] is probably ready to go; with Ard's suggested by
tag [1]. Again, I'm not happy about it; but KernelCI coverage is
ultimately more important to the project.
[0] https://github.com/ClangBuiltLinux/linux/issues/283#issuecomment-457093369
[1] https://github.com/ClangBuiltLinux/linux/issues/283#issuecomment-457087288
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: crypto: Disable xor-neon build when using clang
2019-02-13 21:45 ` Nick Desaulniers
@ 2019-02-14 16:30 ` Mark Brown
2019-02-14 17:00 ` Nathan Chancellor
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2019-02-14 16:30 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Ard Biesheuvel, Catalin Marinas, Jackie Liu, Will Deacon,
Stephen Hines, Kevin Hilman, Nathan Chancellor, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 281 bytes --]
On Wed, Feb 13, 2019 at 01:45:08PM -0800, Nick Desaulniers wrote:
> Nathan's patch [0] is probably ready to go; with Ard's suggested by
> tag [1]. Again, I'm not happy about it; but KernelCI coverage is
> ultimately more important to the project.
OK, Nathan will you post that?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: crypto: Disable xor-neon build when using clang
2019-02-14 16:30 ` Mark Brown
@ 2019-02-14 17:00 ` Nathan Chancellor
0 siblings, 0 replies; 6+ messages in thread
From: Nathan Chancellor @ 2019-02-14 17:00 UTC (permalink / raw)
To: Mark Brown
Cc: Ard Biesheuvel, Catalin Marinas, Jackie Liu, Nick Desaulniers,
Will Deacon, Stephen Hines, Kevin Hilman, linux-arm-kernel
On Thu, Feb 14, 2019 at 04:30:48PM +0000, Mark Brown wrote:
> On Wed, Feb 13, 2019 at 01:45:08PM -0800, Nick Desaulniers wrote:
>
> > Nathan's patch [0] is probably ready to go; with Ard's suggested by
> > tag [1]. Again, I'm not happy about it; but KernelCI coverage is
> > ultimately more important to the project.
>
> OK, Nathan will you post that?
Yes, I can do it later today.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-14 17:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-12 19:46 [PATCH] arm64: crypto: Disable xor-neon build when using clang Mark Brown
2019-02-12 20:33 ` Ard Biesheuvel
2019-02-13 11:40 ` Mark Brown
2019-02-13 21:45 ` Nick Desaulniers
2019-02-14 16:30 ` Mark Brown
2019-02-14 17:00 ` 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).