[PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Nathan Chancellor]

After commit cc9f8349cb33 ("arm64: crypto: add NEON accelerated XOR
implementation"), Clang builds for arm64 started failing with the
following error message.

arch/arm64/lib/xor-neon.c:58:28: error: incompatible pointer types
assigning to 'const unsigned long *' from 'uint64_t *' (aka 'unsigned
long long *') [-Werror,-Wincompatible-pointer-types]
                v3 = veorq_u64(vld1q_u64(dp1 +  6), vld1q_u64(dp2 + 6));
                                         ^~~~~~~~
/usr/lib/llvm-9/lib/clang/9.0.0/include/arm_neon.h:7538:47: note:
expanded from macro 'vld1q_u64'
  __ret = (uint64x2_t) __builtin_neon_vld1q_v(__p0, 51); \
                                              ^~~~

There has been quite a bit of debate and triage that has gone into
figuring out what the proper fix is, viewable at the link below, which
is still ongoing. Ard suggested disabling this warning with Clang with a
pragma so no neon code will have this type of error. While this is not
at all an ideal solution, this build error is the only thing preventing
KernelCI from having successful arm64 defconfig and allmodconfig builds
on linux-next. Getting continuous integration running is more important
so new warnings/errors or boot failures can be caught and fixed quickly.

Link: https://github.com/ClangBuiltLinux/linux/issues/283
Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 arch/arm64/include/asm/neon-intrinsics.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/include/asm/neon-intrinsics.h b/arch/arm64/include/asm/neon-intrinsics.h
index 2ba6c6b9541f..71abfc7612b2 100644
--- a/arch/arm64/include/asm/neon-intrinsics.h
+++ b/arch/arm64/include/asm/neon-intrinsics.h
@@ -36,4 +36,8 @@
 #include <arm_neon.h>
 #endif
 
+#ifdef CONFIG_CC_IS_CLANG
+#pragma clang diagnostic ignored "-Wincompatible-pointer-types"
+#endif
+
 #endif /* __ASM_NEON_INTRINSICS_H */
-- 
2.21.0.rc1

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 278 bytes --]

On Thu, Feb 14, 2019 at 06:39:59PM -0700, Nathan Chancellor wrote:
> After commit cc9f8349cb33 ("arm64: crypto: add NEON accelerated XOR
> implementation"), Clang builds for arm64 started failing with the
> following error message.

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Will Deacon]

On Thu, Feb 14, 2019 at 06:39:59PM -0700, Nathan Chancellor wrote:
> After commit cc9f8349cb33 ("arm64: crypto: add NEON accelerated XOR
> implementation"), Clang builds for arm64 started failing with the
> following error message.
> 
> arch/arm64/lib/xor-neon.c:58:28: error: incompatible pointer types
> assigning to 'const unsigned long *' from 'uint64_t *' (aka 'unsigned
> long long *') [-Werror,-Wincompatible-pointer-types]
>                 v3 = veorq_u64(vld1q_u64(dp1 +  6), vld1q_u64(dp2 + 6));
>                                          ^~~~~~~~
> /usr/lib/llvm-9/lib/clang/9.0.0/include/arm_neon.h:7538:47: note:
> expanded from macro 'vld1q_u64'
>   __ret = (uint64x2_t) __builtin_neon_vld1q_v(__p0, 51); \
>                                               ^~~~
> 
> There has been quite a bit of debate and triage that has gone into
> figuring out what the proper fix is, viewable at the link below, which
> is still ongoing. Ard suggested disabling this warning with Clang with a
> pragma so no neon code will have this type of error. While this is not
> at all an ideal solution, this build error is the only thing preventing
> KernelCI from having successful arm64 defconfig and allmodconfig builds
> on linux-next. Getting continuous integration running is more important
> so new warnings/errors or boot failures can be caught and fixed quickly.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/283
> Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  arch/arm64/include/asm/neon-intrinsics.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/neon-intrinsics.h b/arch/arm64/include/asm/neon-intrinsics.h
> index 2ba6c6b9541f..71abfc7612b2 100644
> --- a/arch/arm64/include/asm/neon-intrinsics.h
> +++ b/arch/arm64/include/asm/neon-intrinsics.h
> @@ -36,4 +36,8 @@
>  #include <arm_neon.h>
>  #endif
>  
> +#ifdef CONFIG_CC_IS_CLANG
> +#pragma clang diagnostic ignored "-Wincompatible-pointer-types"
> +#endif

I'd like Ard's ack on this one, please.

Will

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Ard Biesheuvel]

On Fri, 15 Feb 2019 at 19:50, Will Deacon <will.deacon@arm.com> wrote:
>
> On Thu, Feb 14, 2019 at 06:39:59PM -0700, Nathan Chancellor wrote:
> > After commit cc9f8349cb33 ("arm64: crypto: add NEON accelerated XOR
> > implementation"), Clang builds for arm64 started failing with the
> > following error message.
> >
> > arch/arm64/lib/xor-neon.c:58:28: error: incompatible pointer types
> > assigning to 'const unsigned long *' from 'uint64_t *' (aka 'unsigned
> > long long *') [-Werror,-Wincompatible-pointer-types]
> >                 v3 = veorq_u64(vld1q_u64(dp1 +  6), vld1q_u64(dp2 + 6));
> >                                          ^~~~~~~~
> > /usr/lib/llvm-9/lib/clang/9.0.0/include/arm_neon.h:7538:47: note:
> > expanded from macro 'vld1q_u64'
> >   __ret = (uint64x2_t) __builtin_neon_vld1q_v(__p0, 51); \
> >                                               ^~~~
> >
> > There has been quite a bit of debate and triage that has gone into
> > figuring out what the proper fix is, viewable at the link below, which
> > is still ongoing. Ard suggested disabling this warning with Clang with a
> > pragma so no neon code will have this type of error. While this is not
> > at all an ideal solution, this build error is the only thing preventing
> > KernelCI from having successful arm64 defconfig and allmodconfig builds
> > on linux-next. Getting continuous integration running is more important
> > so new warnings/errors or boot failures can be caught and fixed quickly.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/283
> > Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >  arch/arm64/include/asm/neon-intrinsics.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/neon-intrinsics.h b/arch/arm64/include/asm/neon-intrinsics.h
> > index 2ba6c6b9541f..71abfc7612b2 100644
> > --- a/arch/arm64/include/asm/neon-intrinsics.h
> > +++ b/arch/arm64/include/asm/neon-intrinsics.h
> > @@ -36,4 +36,8 @@
> >  #include <arm_neon.h>
> >  #endif
> >
> > +#ifdef CONFIG_CC_IS_CLANG
> > +#pragma clang diagnostic ignored "-Wincompatible-pointer-types"
> > +#endif
>
> I'd like Ard's ack on this one, please.
>

Provided that we stop sending Clang enablement patches to -stable:

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Nick Desaulniers]

On Fri, Feb 15, 2019 at 11:20 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> Provided that we stop sending Clang enablement patches to -stable:

What does that mean?  We're trying to provide clang support back to
4.4 LTS branches. (so 4.4, 4.9, 4.14, 4.19).
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Ard Biesheuvel]

On Fri, 15 Feb 2019 at 20:25, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Fri, Feb 15, 2019 at 11:20 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> > Provided that we stop sending Clang enablement patches to -stable:
>
> What does that mean?  We're trying to provide clang support back to
> 4.4 LTS branches. (so 4.4, 4.9, 4.14, 4.19).

I understand that is what you are attempting, but that does not mean
it /belongs/ in -stable.

There are rules for stable, and people that track stable kernels (such
as the distros) should be able to rely on us to only backport bug
fixes, not linker script changes and other updates that fix issues
that did not exist when those kernels were released.

It is unclear to me how these clang changes benefit those users.

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Nick Desaulniers]

On Fri, Feb 15, 2019 at 11:28 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On Fri, 15 Feb 2019 at 20:25, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Fri, Feb 15, 2019 at 11:20 AM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > > Provided that we stop sending Clang enablement patches to -stable:
> >
> > What does that mean?  We're trying to provide clang support back to
> > 4.4 LTS branches. (so 4.4, 4.9, 4.14, 4.19).
>
> I understand that is what you are attempting, but that does not mean
> it /belongs/ in -stable.
>
> There are rules for stable, and people that track stable kernels (such
> as the distros) should be able to rely on us to only backport bug
> fixes, not linker script changes and other updates that fix issues
> that did not exist when those kernels were released.
>
> It is unclear to me how these clang changes benefit those users.

If you're referring to
https://www.spinics.net/lists/stable/msg278381.html, that's fair (I
think those were helpful for LLD support on arm64).

Why didn't you speak up then?  Why is this coming up now?
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Ard Biesheuvel]

On Fri, 15 Feb 2019 at 20:43, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Fri, Feb 15, 2019 at 11:28 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > On Fri, 15 Feb 2019 at 20:25, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > On Fri, Feb 15, 2019 at 11:20 AM Ard Biesheuvel
> > > <ard.biesheuvel@linaro.org> wrote:
> > > > Provided that we stop sending Clang enablement patches to -stable:
> > >
> > > What does that mean?  We're trying to provide clang support back to
> > > 4.4 LTS branches. (so 4.4, 4.9, 4.14, 4.19).
> >
> > I understand that is what you are attempting, but that does not mean
> > it /belongs/ in -stable.
> >
> > There are rules for stable, and people that track stable kernels (such
> > as the distros) should be able to rely on us to only backport bug
> > fixes, not linker script changes and other updates that fix issues
> > that did not exist when those kernels were released.
> >
> > It is unclear to me how these clang changes benefit those users.
>
> If you're referring to
> https://www.spinics.net/lists/stable/msg278381.html, that's fair (I
> think those were helpful for LLD support on arm64).
>
> Why didn't you speak up then?  Why is this coming up now?

That is just one example, and I failed to realise it at the time.

I think the Clang/LLVM work you are doing is very important, but I
simply don't think any of it belongs in -stable kernels.

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Ard Biesheuvel]

On Fri, 15 Feb 2019 at 20:45, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Fri, 15 Feb 2019 at 20:43, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Fri, Feb 15, 2019 at 11:28 AM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > >
> > > On Fri, 15 Feb 2019 at 20:25, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > >
> > > > On Fri, Feb 15, 2019 at 11:20 AM Ard Biesheuvel
> > > > <ard.biesheuvel@linaro.org> wrote:
> > > > > Provided that we stop sending Clang enablement patches to -stable:
> > > >
> > > > What does that mean?  We're trying to provide clang support back to
> > > > 4.4 LTS branches. (so 4.4, 4.9, 4.14, 4.19).
> > >
> > > I understand that is what you are attempting, but that does not mean
> > > it /belongs/ in -stable.
> > >
> > > There are rules for stable, and people that track stable kernels (such
> > > as the distros) should be able to rely on us to only backport bug
> > > fixes, not linker script changes and other updates that fix issues
> > > that did not exist when those kernels were released.
> > >
> > > It is unclear to me how these clang changes benefit those users.
> >
> > If you're referring to
> > https://www.spinics.net/lists/stable/msg278381.html, that's fair (I
> > think those were helpful for LLD support on arm64).
> >
> > Why didn't you speak up then?  Why is this coming up now?
>
> That is just one example, and I failed to realise it at the time.
>
> I think the Clang/LLVM work you are doing is very important, but I
> simply don't think any of it belongs in -stable kernels.

OK, to clarify my position:

I have no problem whatsoever with taking this patch into v5.x, so

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

but going forward, I will push back on -stable backports for
Clang/LLVM specific changes, since they are obviously in violation of
the stable kernel rules.

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Greg KH]

On Tue, Feb 19, 2019 at 11:35:12AM +0100, Ard Biesheuvel wrote:
> On Fri, 15 Feb 2019 at 20:45, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > On Fri, 15 Feb 2019 at 20:43, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > On Fri, Feb 15, 2019 at 11:28 AM Ard Biesheuvel
> > > <ard.biesheuvel@linaro.org> wrote:
> > > >
> > > > On Fri, 15 Feb 2019 at 20:25, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > > >
> > > > > On Fri, Feb 15, 2019 at 11:20 AM Ard Biesheuvel
> > > > > <ard.biesheuvel@linaro.org> wrote:
> > > > > > Provided that we stop sending Clang enablement patches to -stable:
> > > > >
> > > > > What does that mean?  We're trying to provide clang support back to
> > > > > 4.4 LTS branches. (so 4.4, 4.9, 4.14, 4.19).
> > > >
> > > > I understand that is what you are attempting, but that does not mean
> > > > it /belongs/ in -stable.
> > > >
> > > > There are rules for stable, and people that track stable kernels (such
> > > > as the distros) should be able to rely on us to only backport bug
> > > > fixes, not linker script changes and other updates that fix issues
> > > > that did not exist when those kernels were released.
> > > >
> > > > It is unclear to me how these clang changes benefit those users.
> > >
> > > If you're referring to
> > > https://www.spinics.net/lists/stable/msg278381.html, that's fair (I
> > > think those were helpful for LLD support on arm64).
> > >
> > > Why didn't you speak up then?  Why is this coming up now?
> >
> > That is just one example, and I failed to realise it at the time.
> >
> > I think the Clang/LLVM work you are doing is very important, but I
> > simply don't think any of it belongs in -stable kernels.
> 
> OK, to clarify my position:
> 
> I have no problem whatsoever with taking this patch into v5.x, so
> 
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> but going forward, I will push back on -stable backports for
> Clang/LLVM specific changes, since they are obviously in violation of
> the stable kernel rules.

Getting older kernels to build/run properly on newer compilers is just a
part of life for the stable trees.  If you note, we have done a lot of
gcc7, then gcc8, and clang patches backported over the years in order to
make it possible for people (like me and my testing infrastructure at
the least) to keep building these old kernels on newer systems.

So while it's not part of the "documented" rules, I do take this type of
change, as it does help out a huge population of users and testers.

thanks,

greg k-h

Re: [PATCH] arm64/neon: Disable -Wincompatible-pointer-types when building with Clang

[Ard Biesheuvel]

On Tue, 19 Feb 2019 at 12:19, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Feb 19, 2019 at 11:35:12AM +0100, Ard Biesheuvel wrote:
> > On Fri, 15 Feb 2019 at 20:45, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > >
> > > On Fri, 15 Feb 2019 at 20:43, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > >
> > > > On Fri, Feb 15, 2019 at 11:28 AM Ard Biesheuvel
> > > > <ard.biesheuvel@linaro.org> wrote:
> > > > >
> > > > > On Fri, 15 Feb 2019 at 20:25, Nick Desaulniers <ndesaulniers@google.com> wrote:
> > > > > >
> > > > > > On Fri, Feb 15, 2019 at 11:20 AM Ard Biesheuvel
> > > > > > <ard.biesheuvel@linaro.org> wrote:
> > > > > > > Provided that we stop sending Clang enablement patches to -stable:
> > > > > >
> > > > > > What does that mean?  We're trying to provide clang support back to
> > > > > > 4.4 LTS branches. (so 4.4, 4.9, 4.14, 4.19).
> > > > >
> > > > > I understand that is what you are attempting, but that does not mean
> > > > > it /belongs/ in -stable.
> > > > >
> > > > > There are rules for stable, and people that track stable kernels (such
> > > > > as the distros) should be able to rely on us to only backport bug
> > > > > fixes, not linker script changes and other updates that fix issues
> > > > > that did not exist when those kernels were released.
> > > > >
> > > > > It is unclear to me how these clang changes benefit those users.
> > > >
> > > > If you're referring to
> > > > https://www.spinics.net/lists/stable/msg278381.html, that's fair (I
> > > > think those were helpful for LLD support on arm64).
> > > >
> > > > Why didn't you speak up then?  Why is this coming up now?
> > >
> > > That is just one example, and I failed to realise it at the time.
> > >
> > > I think the Clang/LLVM work you are doing is very important, but I
> > > simply don't think any of it belongs in -stable kernels.
> >
> > OK, to clarify my position:
> >
> > I have no problem whatsoever with taking this patch into v5.x, so
> >
> > Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > but going forward, I will push back on -stable backports for
> > Clang/LLVM specific changes, since they are obviously in violation of
> > the stable kernel rules.
>
> Getting older kernels to build/run properly on newer compilers is just a
> part of life for the stable trees.  If you note, we have done a lot of
> gcc7, then gcc8, and clang patches backported over the years in order to
> make it possible for people (like me and my testing infrastructure at
> the least) to keep building these old kernels on newer systems.
>
> So while it's not part of the "documented" rules, I do take this type of
> change, as it does help out a huge population of users and testers.
>

That doesn't make a lot of sense to me, but that is the common
practice, then I won't object any longer.