[PATCH v3 RESEND 0/2] xor-neon: Remove GCC warn & pragmas

[PATCH v3 RESEND 0/2] xor-neon: Remove GCC warn & pragmas

[Adrian Ratiu]

Dear all,

This is a resend of v3 of the patch series started at
id:20201106051436.2384842-1-adrian.ratiu@collabora.com

This series does not address the Clang -ftree-vectorize not
working bug which is a known pre-existing issued documented
at [1] [2] [3]. Clang vectorization needs to be investigated
in more deepth and fixed separately. The purpouse of this is
to only fix some low-hanging-fruit GCC related isues.

Tested on next-20210118 using GCC 10.2.0 and Clang 10.0.1.

[1] https://bugs.llvm.org/show_bug.cgi?id=40976
[2] https://github.com/ClangBuiltLinux/linux/issues/503
[3] https://github.com/ClangBuiltLinux/linux/issues/496

Kind regards,
Adrian

Adrian Ratiu (1):
  arm: lib: xor-neon: move pragma options to makefile

Nathan Chancellor (1):
  arm: lib: xor-neon: remove unnecessary GCC < 4.6 warning

 arch/arm/lib/Makefile   |  2 +-
 arch/arm/lib/xor-neon.c | 17 -----------------
 2 files changed, 1 insertion(+), 18 deletions(-)

-- 
2.30.0

[PATCH v3 RESEND 1/2] arm: lib: xor-neon: remove unnecessary GCC < 4.6 warning

[Adrian Ratiu]

From: Nathan Chancellor <natechancellor@gmail.com>

Drop warning because kernel now requires GCC >= v4.9 after
commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
and clarify that -ftree-vectorize now always needs enabling
for GCC by directly testing the presence of CONFIG_CC_IS_GCC.

Another reason to remove the warning is that Clang exposes
itself as GCC < 4.6 so it triggers the warning about GCC
which doesn't make much sense and risks misleading users.

As a side-note remark, -fttree-vectorize is on by default in
Clang, but it currently does not work (see linked issues).

Link: https://github.com/ClangBuiltLinux/linux/issues/496
Link: https://github.com/ClangBuiltLinux/linux/issues/503
Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 arch/arm/lib/xor-neon.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/arm/lib/xor-neon.c b/arch/arm/lib/xor-neon.c
index b99dd8e1c93f..e1e76186ec23 100644
--- a/arch/arm/lib/xor-neon.c
+++ b/arch/arm/lib/xor-neon.c
@@ -19,15 +19,8 @@ MODULE_LICENSE("GPL");
  * -ftree-vectorize) to attempt to exploit implicit parallelism and emit
  * NEON instructions.
  */
-#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+#ifdef CONFIG_CC_IS_GCC
 #pragma GCC optimize "tree-vectorize"
-#else
-/*
- * While older versions of GCC do not generate incorrect code, they fail to
- * recognize the parallel nature of these functions, and emit plain ARM code,
- * which is known to be slower than the optimized ARM code in asm-arm/xor.h.
- */
-#warning This code requires at least version 4.6 of GCC
 #endif
 
 #pragma GCC diagnostic ignored "-Wunused-variable"
-- 
2.30.0

[PATCH v3 RESEND 2/2] arm: lib: xor-neon: move pragma options to makefile

[Adrian Ratiu]

Using a pragma like GCC optimize is a bad idea because it tags
all functions with an __attribute__((optimize)) which replaces
optimization options rather than appending so could result in
dropping important flags. Not recommended for production use.

Because these options should always be enabled for this file,
it's better to set them via command line. tree-vectorize is on
by default in Clang, but it doesn't hurt to make it explicit.

Suggested-by: Arvind Sankar <nivedita@alum.mit.edu>
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 arch/arm/lib/Makefile   |  2 +-
 arch/arm/lib/xor-neon.c | 10 ----------
 2 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 6d2ba454f25b..12d31d1a7630 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -45,6 +45,6 @@ $(obj)/csumpartialcopyuser.o:	$(obj)/csumpartialcopygeneric.S
 
 ifeq ($(CONFIG_KERNEL_MODE_NEON),y)
   NEON_FLAGS			:= -march=armv7-a -mfloat-abi=softfp -mfpu=neon
-  CFLAGS_xor-neon.o		+= $(NEON_FLAGS)
+  CFLAGS_xor-neon.o		+= $(NEON_FLAGS) -ftree-vectorize -Wno-unused-variable
   obj-$(CONFIG_XOR_BLOCKS)	+= xor-neon.o
 endif
diff --git a/arch/arm/lib/xor-neon.c b/arch/arm/lib/xor-neon.c
index e1e76186ec23..62b493e386c4 100644
--- a/arch/arm/lib/xor-neon.c
+++ b/arch/arm/lib/xor-neon.c
@@ -14,16 +14,6 @@ MODULE_LICENSE("GPL");
 #error You should compile this file with '-march=armv7-a -mfloat-abi=softfp -mfpu=neon'
 #endif
 
-/*
- * Pull in the reference implementations while instructing GCC (through
- * -ftree-vectorize) to attempt to exploit implicit parallelism and emit
- * NEON instructions.
- */
-#ifdef CONFIG_CC_IS_GCC
-#pragma GCC optimize "tree-vectorize"
-#endif
-
-#pragma GCC diagnostic ignored "-Wunused-variable"
 #include <asm-generic/xor.h>
 
 struct xor_block_template const xor_block_neon_inner = {
-- 
2.30.0

Re: [PATCH v3 RESEND 1/2] arm: lib: xor-neon: remove unnecessary GCC < 4.6 warning

[Arnd Bergmann]

On Mon, Jan 18, 2021 at 11:56 AM Adrian Ratiu
<adrian.ratiu@collabora.com> wrote:
>
> From: Nathan Chancellor <natechancellor@gmail.com>
>
> Drop warning because kernel now requires GCC >= v4.9 after
> commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
> and clarify that -ftree-vectorize now always needs enabling
> for GCC by directly testing the presence of CONFIG_CC_IS_GCC.
>
> Another reason to remove the warning is that Clang exposes
> itself as GCC < 4.6 so it triggers the warning about GCC
> which doesn't make much sense and risks misleading users.
>
> As a side-note remark, -fttree-vectorize is on by default in
> Clang, but it currently does not work (see linked issues).
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/496
> Link: https://github.com/ClangBuiltLinux/linux/issues/503
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>

Shouldn't there be a check for whatever minimum version of clang
produces optimized code now? As I understand it, the warning
was originally meant to complain about both old gcc and any
version of clang, while waiting for a new version of clang to
produce vectorized code.

Has that happened now?

       Arnd

Re: [PATCH v3 RESEND 1/2] arm: lib: xor-neon: remove unnecessary GCC < 4.6 warning

[Adrian Ratiu]

On Mon, 18 Jan 2021, Arnd Bergmann <arnd@kernel.org> wrote:
> On Mon, Jan 18, 2021 at 11:56 AM Adrian Ratiu 
> <adrian.ratiu@collabora.com> wrote: 
>> 
>> From: Nathan Chancellor <natechancellor@gmail.com> 
>> 
>> Drop warning because kernel now requires GCC >= v4.9 after 
>> commit 6ec4476ac825 ("Raise gcc version requirement to 4.9") 
>> and clarify that -ftree-vectorize now always needs enabling for 
>> GCC by directly testing the presence of CONFIG_CC_IS_GCC. 
>> 
>> Another reason to remove the warning is that Clang exposes 
>> itself as GCC < 4.6 so it triggers the warning about GCC which 
>> doesn't make much sense and risks misleading users. 
>> 
>> As a side-note remark, -fttree-vectorize is on by default in 
>> Clang, but it currently does not work (see linked issues). 
>> 
>> Link: https://github.com/ClangBuiltLinux/linux/issues/496 Link: 
>> https://github.com/ClangBuiltLinux/linux/issues/503 
>> Reported-by: Nick Desaulniers <ndesaulniers@google.com> 
>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> 
>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> 
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com> 
> 
> Shouldn't there be a check for whatever minimum version of clang 
> produces optimized code now? As I understand it, the warning was 
> originally meant to complain about both old gcc and any version 
> of clang, while waiting for a new version of clang to produce 
> vectorized code. 
> 
> Has that happened now? 

No, clang does not produce vectorized code by default, not even 
with the -ftree-vectorize flag explicitely added like in the next 
patch in this series (that flag is enabled by default in clang 
anyway, so no effect).

Clang needs more investigation and testing because with additional 
code changes it can be "forced" to output vectorized code, but 
that is outside the scope of this series.

If you think it's a good idea I can add a warning only for Clang 
which makes more sense than telling clang users to upgrade their 
GCC, since now Clang is officially supported. What do you think?


>
>        Arnd

Re: [PATCH v3 RESEND 1/2] arm: lib: xor-neon: remove unnecessary GCC < 4.6 warning

[Arnd Bergmann]

On Mon, Jan 18, 2021 at 12:28 PM Adrian Ratiu
<adrian.ratiu@collabora.com> wrote:
> On Mon, 18 Jan 2021, Arnd Bergmann <arnd@kernel.org> wrote:
> > On Mon, Jan 18, 2021 at 11:56 AM Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
>
> No, clang does not produce vectorized code by default, not even
> with the -ftree-vectorize flag explicitely added like in the next
> patch in this series (that flag is enabled by default in clang
> anyway, so no effect).
>
> Clang needs more investigation and testing because with additional
> code changes it can be "forced" to output vectorized code, but
> that is outside the scope of this series.
>
> If you think it's a good idea I can add a warning only for Clang
> which makes more sense than telling clang users to upgrade their
> GCC, since now Clang is officially supported. What do you think?

Yes, either a warning or a Kconfig check seems better to me than
just trying to build code that ends up not doing what it is meant to.

       Arnd