| Message ID | 20190215013959.21320-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Commit | 0738c8b5915c7eaf1e6007b441008e8f3b460443 |
| Headers | show |
| Series | arm64/neon: Disable -Wincompatible-pointer-types when building with Clang | expand |
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>
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
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>
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).
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.
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?
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.
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.
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
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.
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 */
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(+)