-Wnonportable-vector-initialization in crypto/aegis128-neon-inner.c

[nathanchance]

crypto/aegis128-neon-inner.c:58:40: warning: vector initializers are not compatible with NEON intrinsics in big endian mode [-Wnonportable-vector-initialization]
                static const uint8x16_t shift_rows = {
                                                     ^
crypto/aegis128-neon-inner.c:58:40: note: consider using vld1q_u8() to initialize a vector from memory, or vcombine_u8(vcreate_u8(), vcreate_u8()) to initialize from integer constants
crypto/aegis128-neon-inner.c:62:38: warning: vector initializers are not compatible with NEON intrinsics in big endian mode [-Wnonportable-vector-initialization]
                static const uint8x16_t ror32by8 = {
                                                   ^
crypto/aegis128-neon-inner.c:62:38: note: consider using vld1q_u8() to initialize a vector from memory, or vcombine_u8(vcreate_u8(), vcreate_u8()) to initialize from integer constants
2 warnings generated.

Pops up in arm64 all{mod,yes}config builds.

Caused by https://git.kernel.org/next/linux-next/c/198429631a85622da1d08d360ef02cfb84c95919

cc @ardbiesheuvel

[ardbiesheuvel]

Does this help?

--- a/crypto/aegis128-neon-inner.c
+++ b/crypto/aegis128-neon-inner.c
@@ -55,14 +55,15 @@ uint8x16_t aegis_aes_round(uint8x16_t w)
 
 #ifdef CONFIG_ARM64
        if (!__builtin_expect(aegis128_have_aes_insn, 1)) {
-               static const uint8x16_t shift_rows = {
+               const uint8x16_t shift_rows = vld1q_u8((u8 []){
                        0x0, 0x5, 0xa, 0xf, 0x4, 0x9, 0xe, 0x3,
                        0x8, 0xd, 0x2, 0x7, 0xc, 0x1, 0x6, 0xb,
-               };
-               static const uint8x16_t ror32by8 = {
+               });
+               const uint8x16_t ror32by8 = vld1q_u8((u8 []){
                        0x1, 0x2, 0x3, 0x0, 0x5, 0x6, 0x7, 0x4,
                        0x9, 0xa, 0xb, 0x8, 0xd, 0xe, 0xf, 0xc,
-               };
+               });
                uint8x16_t v;
 
                // shift rows

[nathanchance]

That diff has this error:

crypto/aegis128-neon-inner.c:59:9: error: too many arguments provided to function-like macro invocation
                        0x0, 0x5, 0xa, 0xf, 0x4, 0x9, 0xe, 0x3,
                             ^
/home/nathan/cbl/usr/lib/clang/10.0.0/include/arm_neon.h:7512:9: note: macro 'vld1q_u8' defined here
#define vld1q_u8(__p0) __extension__ ({ \
        ^
crypto/aegis128-neon-inner.c:58:33: note: parentheses are required around macro argument containing braced initializer list
                const uint8x16_t shift_rows = vld1q_u8((u8 []){
                                              ^
                                                       (
crypto/aegis128-neon-inner.c:63:9: error: too many arguments provided to function-like macro invocation
                        0x1, 0x2, 0x3, 0x0, 0x5, 0x6, 0x7, 0x4,
                             ^
/home/nathan/cbl/usr/lib/clang/10.0.0/include/arm_neon.h:7512:9: note: macro 'vld1q_u8' defined here
#define vld1q_u8(__p0) __extension__ ({ \
        ^
crypto/aegis128-neon-inner.c:62:31: note: parentheses are required around macro argument containing braced initializer list
                const uint8x16_t ror32by8 = vld1q_u8((u8 []){
                                            ^
                                                     (
2 errors generated.

This works:

diff --git a/crypto/aegis128-neon-inner.c b/crypto/aegis128-neon-inner.c
index ed55568afd1b..236c2e806470 100644
--- a/crypto/aegis128-neon-inner.c
+++ b/crypto/aegis128-neon-inner.c
@@ -55,14 +55,14 @@ uint8x16_t aegis_aes_round(uint8x16_t w)
 
 #ifdef CONFIG_ARM64
        if (!__builtin_expect(aegis128_have_aes_insn, 1)) {
-               static const uint8x16_t shift_rows = {
+               const uint8x16_t shift_rows = vld1q_u8(((u8 []){
                        0x0, 0x5, 0xa, 0xf, 0x4, 0x9, 0xe, 0x3,
                        0x8, 0xd, 0x2, 0x7, 0xc, 0x1, 0x6, 0xb,
-               };
-               static const uint8x16_t ror32by8 = {
+               }));
+               const uint8x16_t ror32by8 = vld1q_u8(((u8 []){
                        0x1, 0x2, 0x3, 0x0, 0x5, 0x6, 0x7, 0x4,
                        0x9, 0xa, 0xb, 0x8, 0xd, 0xe, 0xf, 0xc,
-               };
+               }));
                uint8x16_t v;
 
                // shift rows

[ardbiesheuvel]

OK, I have sent another fix here:
https://lore.kernel.org/linux-crypto/20190819141500.1070-1-ard.biesheuvel@linaro.org/T/#u

However, the Clang build error you are getting is an issue that Clang needs to fix: for some reason, they decided to use C preprocessor macros to implement vld1q_u8() et al, but the designated initializer (e.g., '(u8[]){1,2,3}') is valid C syntax, and so the compiler should not choke on it.

[nickdesaulniers]

but the designated initializer (e.g., '(u8[]){1,2,3}') is valid C syntax, and so the compiler should not choke on it.

Oh, that's indeed an interesting case. Let me tease out a reproducer more. (C++20 adding support for designated initializers is sure to make this a larger issue at some point).

[nathanchance]

Fixed in next-20190903: https://git.kernel.org/next/linux-next/c/389139b34f407da7c09bc26c4d943f52742a6d42

@nickdesaulniers do you want to leave this open to look at the designated initializer issue or open a new issue for that?

[nickdesaulniers]

Leave open; I'll file a bug upstream.

[nickdesaulniers]

However, the Clang build error you are getting is an issue that Clang needs to fix: for some reason, they decided to use C preprocessor macros to implement vld1q_u8() et al, but the designated initializer (e.g., '(u8[]){1,2,3}') is valid C syntax, and so the compiler should not choke on it.

Filed: https://llvm.org/pr43331

[nickdesaulniers]

The upstream issue cross references this one. If there's any movement there, I'll reopen this one or comment further.