incomplete .arch specifiers with latest Linaro gcc5 patches on armv8?

Discussion:

Robert Schiele

2015-12-04 14:13:44 UTC

Hi,

when working with the Linaro patches I found that a particular commit
breaks our aarch64 kernel build.

The patch in question is that one:

commit be09330da9d0777c4a58568d137e3f8a3dbe0a0b
Author: Yvan Roux <***@linaro.org>
Date: Tue Oct 27 21:18:19 2015 +0100

One of the things it attempts to change apparently is moving the .arch
specifiers in the assembler file from a global scope to individual
functions. What also happens though is that they seem to lose some
information after that transformation.

I observed that when building arch/arm64/crypto/aes-ce-cipher.c from
the Linux kernel. This code contains inline assembly like this:

static void aes_cipher_decrypt(struct crypto_tfm *tfm, u8 dst[], u8 const src[])
{
struct crypto_aes_ctx *ctx = crypto_tfm_ctx(tfm);
struct aes_block *out = (struct aes_block *)dst;
struct aes_block const *in = (struct aes_block *)src;
void *dummy0;
int dummy1;

kernel_neon_begin_partial(4);

__asm__(" ld1 {v0.16b}, %[in] ;"
" ld1 {v1.2d}, [%[key]], #16 ;"
" cmp %w[rounds], #10 ;"
" bmi 0f ;"
" bne 3f ;"
" mov v3.16b, v1.16b ;"
" b 2f ;"
"0: mov v2.16b, v1.16b ;"
" ld1 {v3.2d}, [%[key]], #16 ;"
"1: aesd v0.16b, v2.16b ;"
" aesimc v0.16b, v0.16b ;"
"2: ld1 {v1.2d}, [%[key]], #16 ;"
" aesd v0.16b, v3.16b ;"
" aesimc v0.16b, v0.16b ;"
"3: ld1 {v2.2d}, [%[key]], #16 ;"
" subs %w[rounds], %w[rounds], #3 ;"
" aesd v0.16b, v1.16b ;"
" aesimc v0.16b, v0.16b ;"
" ld1 {v3.2d}, [%[key]], #16 ;"
" bpl 1b ;"
" aesd v0.16b, v2.16b ;"
" eor v0.16b, v0.16b, v3.16b ;"
" st1 {v0.16b}, %[out] ;"

: [out] "=Q"(*out),
[key] "=r"(dummy0),
[rounds] "=r"(dummy1)
: [in] "Q"(*in),
"1"(ctx->key_dec),
"2"(num_rounds(ctx) - 2)
: "cc");

kernel_neon_end();
}

Now without this patch the compiler behaved like the following. It was
invoked with:

aarch64-linux-gnu-gcc -Wp,-MD,arch/arm64/crypto/.aes-ce-cipher.o.d
-nostdinc -isystem
/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/bin/../lib/gcc/aarch64-linux-gnu/5.2.1/include
-I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/arch/arm64/include
-Iarch/arm64/include/generated/uapi -Iarch/arm64/include/generated
-I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/include
-Iinclude -I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/arch/arm64/include/uapi
-Iarch/arm64/include/generated/uapi
-I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/include/uapi
-Iinclude/generated/uapi -include
/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/include/linux/kconfig.h
-I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/arch/arm64/crypto
-Iarch/arm64/crypto -D__KERNEL__ -mlittle-endian -Wall -Wundef
-Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common
-Werror-implicit-function-declaration -Wno-format-security -std=gnu89
-mgeneral-regs-only -fno-delete-null-pointer-checks -O2
--param=allow-store-data-races=0 -Wframe-larger-than=2048
-fno-stack-protector -Wno-unused-but-set-variable
-fno-omit-frame-pointer -fno-optimize-sibling-calls
-fno-var-tracking-assignments -g -Wdeclaration-after-statement
-Wno-pointer-sign -fno-strict-overflow -fconserve-stack
-Werror=implicit-int -Werror=strict-prototypes -Werror=date-time
-DCC_HAVE_ASM_GOTO -Werror -march=armv8-a+crypto
-D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(aes_ce_cipher)"
-D"KBUILD_MODNAME=KBUILD_STR(aes_ce_cipher)" -c -o
arch/arm64/crypto/aes-ce-cipher.o
/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/arch/arm64/crypto/aes-ce-cipher.c

As a result it created a file for the assembler with the global

.arch armv8-a+fp+simd+crypto

at the beginning of the file.

After the patch it created individual

.arch armv8-a

at individual places.

It is not clear to me, why the extensions (fp+simd+crypto) got lost.
Is that intended, such that the code needs special adaption for inline
assembly using those extensions or is that loss of extensions a bug of
that patch?

Greetings!
Robert

Yvan Roux

2015-12-04 15:47:54 UTC

Permalink

Hi Robert,

Post by Robert Schiele
Hi,
when working with the Linaro patches I found that a particular commit
breaks our aarch64 kernel build.
commit be09330da9d0777c4a58568d137e3f8a3dbe0a0b
Date: Tue Oct 27 21:18:19 2015 +0100
One of the things it attempts to change apparently is moving the .arch
specifiers in the assembler file from a global scope to individual
functions. What also happens though is that they seem to lose some
information after that transformation.
I observed that when building arch/arm64/crypto/aes-ce-cipher.c from
static void aes_cipher_decrypt(struct crypto_tfm *tfm, u8 dst[], u8 const src[])
{
struct crypto_aes_ctx *ctx = crypto_tfm_ctx(tfm);
struct aes_block *out = (struct aes_block *)dst;
struct aes_block const *in = (struct aes_block *)src;
void *dummy0;
int dummy1;
kernel_neon_begin_partial(4);
__asm__(" ld1 {v0.16b}, %[in] ;"
" ld1 {v1.2d}, [%[key]], #16 ;"
" cmp %w[rounds], #10 ;"
" bmi 0f ;"
" bne 3f ;"
" mov v3.16b, v1.16b ;"
" b 2f ;"
"0: mov v2.16b, v1.16b ;"
" ld1 {v3.2d}, [%[key]], #16 ;"
"1: aesd v0.16b, v2.16b ;"
" aesimc v0.16b, v0.16b ;"
"2: ld1 {v1.2d}, [%[key]], #16 ;"
" aesd v0.16b, v3.16b ;"
" aesimc v0.16b, v0.16b ;"
"3: ld1 {v2.2d}, [%[key]], #16 ;"
" subs %w[rounds], %w[rounds], #3 ;"
" aesd v0.16b, v1.16b ;"
" aesimc v0.16b, v0.16b ;"
" ld1 {v3.2d}, [%[key]], #16 ;"
" bpl 1b ;"
" aesd v0.16b, v2.16b ;"
" eor v0.16b, v0.16b, v3.16b ;"
" st1 {v0.16b}, %[out] ;"
: [out] "=Q"(*out),
[key] "=r"(dummy0),
[rounds] "=r"(dummy1)
: [in] "Q"(*in),
"1"(ctx->key_dec),
"2"(num_rounds(ctx) - 2)
: "cc");
kernel_neon_end();
}
Now without this patch the compiler behaved like the following. It was
aarch64-linux-gnu-gcc -Wp,-MD,arch/arm64/crypto/.aes-ce-cipher.o.d
-nostdinc -isystem
/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/bin/../lib/gcc/aarch64-linux-gnu/5.2.1/include
-I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/arch/arm64/include
-Iarch/arm64/include/generated/uapi -Iarch/arm64/include/generated
-I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/include
-Iinclude -I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/arch/arm64/include/uapi
-Iarch/arm64/include/generated/uapi
-I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/include/uapi
-Iinclude/generated/uapi -include
/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/include/linux/kconfig.h
-I/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/arch/arm64/crypto
-Iarch/arm64/crypto -D__KERNEL__ -mlittle-endian -Wall -Wundef
-Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common
-Werror-implicit-function-declaration -Wno-format-security -std=gnu89
-mgeneral-regs-only -fno-delete-null-pointer-checks -O2
--param=allow-store-data-races=0 -Wframe-larger-than=2048
-fno-stack-protector -Wno-unused-but-set-variable
-fno-omit-frame-pointer -fno-optimize-sibling-calls
-fno-var-tracking-assignments -g -Wdeclaration-after-statement
-Wno-pointer-sign -fno-strict-overflow -fconserve-stack
-Werror=implicit-int -Werror=strict-prototypes -Werror=date-time
-DCC_HAVE_ASM_GOTO -Werror -march=armv8-a+crypto
-D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(aes_ce_cipher)"
-D"KBUILD_MODNAME=KBUILD_STR(aes_ce_cipher)" -c -o
arch/arm64/crypto/aes-ce-cipher.o
/var/fpwork/rschiele/crossbuild/builds/aarch64-linux-gnu/linux-next/srcdir/src/linux/arch/arm64/crypto/aes-ce-cipher.c
As a result it created a file for the assembler with the global
.arch armv8-a+fp+simd+crypto
at the beginning of the file.
After the patch it created individual
.arch armv8-a
at individual places.
It is not clear to me, why the extensions (fp+simd+crypto) got lost.
Is that intended, such that the code needs special adaption for inline
assembly using those extensions or is that loss of extensions a bug of
that patch?

Hmmm, this seems to be triggered by -mgeneral-regs-only flag, at least
on my side, I've to investigate more to give you a real answer, this
backport is not a small one ;) But do you confirm that if you remove
that flag the .arch are correct ?

Thanks,
Yvan

Post by Robert Schiele
Greetings!
Robert
_______________________________________________
linaro-toolchain mailing list
https://lists.linaro.org/mailman/listinfo/linaro-toolchain

Yvan Roux

2015-12-04 17:57:02 UTC

Permalink

Post by Yvan Roux
Hi Robert,

Kyrill submitted a patch for that issue which was lost in the pile and
approved today:

https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00576.html

I'll backport it in our branch as soon as it is committed on trunk.

Thanks,
Yvan

Robert Schiele

2015-12-04 21:09:57 UTC

Permalink

Hi Yvan,

Post by Yvan Roux

Post by Yvan Roux
Hmmm, this seems to be triggered by -mgeneral-regs-only flag, at least
on my side, I've to investigate more to give you a real answer, this
backport is not a small one ;) But do you confirm that if you remove
that flag the .arch are correct ?

I will check that on Monday when I am back in the office or even better...

Post by Yvan Roux
Kyrill submitted a patch for that issue which was lost in the pile and
https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00576.html
I'll backport it in our branch as soon as it is committed on trunk.

... I will apply that patch to our tree since the description and code
change seems to exactly describe my observed behavior and I will
provide you feedback on the results.

Robert

Robert Schiele

2015-12-07 06:47:57 UTC

Permalink

Hi Yvan,

Post by Robert Schiele

... I will apply that patch to our tree since the description and code
change seems to exactly describe my observed behavior and I will
provide you feedback on the results.

As expected that patch was the perfect cure for the problem I saw.

Robert

Yvan Roux

2015-12-07 09:04:56 UTC

Permalink

Hi Robert,

Post by Robert Schiele
Hi Yvan,

Post by Robert Schiele

... I will apply that patch to our tree since the description and code
change seems to exactly describe my observed behavior and I will
provide you feedback on the results.

As expected that patch was the perfect cure for the problem I saw.

Ok great, it'll be in our branch soon.

Thanks
Yvan

Post by Robert Schiele
Robert