[AArch64] -mgeneral-regs-only inconsistent with gcc

[pirama-arumuga-nainar]

Bugzilla Link	30792
Version	trunk
OS	Linux
Blocks	llvm/llvm-project#4440
CC	@Arnaud-de-Grandmaison-ARM,@efriedma-quic,@gburgessiv,@kbeyls,@slacka,@m-gupta,@nickdesaulniers,@petrhosek,@sbaranga-arm,@stephenhines,@TNorthover

Extended Description

The AArch64 backend runs into a fatal error with the -mgeneral-regs-only when inline assembly refers to a SIMD register.

A reduced test-case:

$ cat > clobber.c << EOF void dummy() { asm volatile ("" ::: "v0"); } EOF

$ clang -c clobber.c -target aarch64-linux-gnu -mgeneral-regs-only fatal error: error in backend: Do not know how to split the result of this operator!

The code is compiled by gcc, though. Seems like gcc is lenient with respect to this flag. From https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html: -mgeneral-regs-only Generate code which uses only the general-purpose registers. This will prevent the compiler from using floating-point and Advanced SIMD registers but will not impose any restrictions on the assembler.

From http://clang.llvm.org/docs/UsersManual.html: -mgeneral-regs-only Generate code which only uses the general purpose registers. This option restricts the generated code to use general registers only. This only applies to the AArch64 architecture.

There are two possible actions here:

1. Match gcc and allow inline assembly to have SIMD registers. This may be hard to do, considering that '-mgeneral-regs-only' just passes '-targe t-feature -fp-armv8 -target-feature -crypto -target-feature -neon' to the driver.

2. Make the driver not crash, and issue an error instead.

This usage seems prevalent in the kernel, which uses -mgeneral-regs-only to avoid saving and restoring the userspace FPSIMD context on every syscall, but hard-codes FPSIMD or crypto instructions in the handful of places they're useful.

[edwintorok]

mentioned in issue llvm/llvm-project#4440

[gburgessiv]

Proposed fix: https://reviews.llvm.org/D38479

[gburgessiv]

Yeah, I have a half-made patch that tries to address this. Was planning on sending it out by the end of next week.

[m-gupta]

George, Are you working on it?

[nickdesaulniers]

For anyone interested in picking this up, this is the last bug preventing us from compiling the upstream Linux kernel with LLVM for aarch64 without any out of tree patches.

For more information, please see:

• https://www.linuxplumbersconf.org/2017/ocw//system/presentations/4799/original/LPC%202017-%20Clang%20built%20kernels.pdf
• https://chromium.googlesource.com/chromiumos/third_party/kernel/+/1760910682ce3750f775452b8f97872b4e3ed808%5E%21/#F0
• https://lkml.org/lkml/2017/8/22/912

[fba5f26d-3e2e-4f9e-8aef-b87df32319ac]

Hi Manoj,

The solution at D26856 was the wrong one, we need to reject FP uses in front-end (so probably D26856 could be abandoned).

I don't have any plans to implement this at the moment.

However, we should have a work-around with -mno-implicit-float (as long as the code doesn't use any floating point types).

Thanks, Silviu

[m-gupta]

Hi Silviu,

Are you planning on fixing this? I see that https://reviews.llvm.org/D26856 has not been updated in a while.

[pirama-arumuga-nainar]

Tim, thanks for the pointer to use -mno-implicit-float. It works for the Kernel because this particular file does not have any explicit floating point operations.

I also agree with the discussion in https://reviews.llvm.org/D26856 that -mgeneral-regs-only should reject explicit floating point code.

[TNorthover]

The only difference between this and -mno-implicit-float (which works without adding extra backend features) seems to be how it handles explicit floating-point code.

-mno-implicit-float allows it; -mgeneral-regs-only errors on GCC (it's even polite enough to apologize!), but tries to form some new horribly unsupported soft-float ABI on Clang.

I'd suggest switching it to set NoImplicitFloat instead to fix this particular issue, and then perhaps working out how it should behave in the front-end (error, == -mfloat-abi=soft, or floats allowed).

[fba5f26d-3e2e-4f9e-8aef-b87df32319ac]

Thanks, Pirama!

Yes, I went through all of the use cases there. One of them has an umov that clang doesn't like, but otherwise they all work.

In that case I'll put the patches up for review.

-Silviu

[pirama-arumuga-nainar]

Hi Silviu, did you try the inline assembly in https://android.googlesource.com/kernel/tegra/+/android-tegra-3.10/arch/arm64/crypto/aes-ce-cipher.c? There are two assembly blocks there that clobber v0 and v1.

Unfortunately, I couldn't find any other example use case.

[fba5f26d-3e2e-4f9e-8aef-b87df32319ac]

Hi Pirama,

I now have a preliminary fix this.

Turning the crypto and neon features on only in the assembler turned out to be an easy thing to do. However, correctly handling the inline assembler constraints is more difficult and error-prone (because the simd registers now hold illegal types).

Do you have other examples where this feature is used? I would like to do some more testing before putting this out for review.

Thanks, Silviu

[pirama-arumuga-nainar]

Hi Silviu, based on my understanding, yes, it'd be really useful from a performance perspective to match gcc, especially if it is straightforward.

https://android.googlesource.com/kernel/tegra/+/android-tegra-3.10/arch/arm64/crypto/aes-ce-cipher.c is one example of such a file. Disabling -mgeneral-regs-only for just these files would still necessitate saving of FPSIMD context. Extracting the inline assembly out to separate files would mean relying on lto (which I am not sure is functional for kernel builds yet.

[fba5f26d-3e2e-4f9e-8aef-b87df32319ac]

Hi Pirama,

I'm trying to figure out what the best action would be for this. Would option 2) work for the main use case (the kernel)?

As you've noted, matching the gcc behaviour would require some effort (although AFAICT it should be straight-forward to do).

Thanks, Silviu

[pirama-arumuga-nainar]

assigned to @gburgessiv

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: None (pirama-arumuga-nainar)

| | | | --- | --- | | Bugzilla Link | [30792](https://llvm.org/bz30792) | | Version | trunk | | OS | Linux | | Blocks | llvm/llvm-project#4440 | | CC | @Arnaud-de-Grandmaison-ARM,@efriedma-quic,@gburgessiv,@kbeyls,@slacka,@m-gupta,@nickdesaulniers,@petrhosek,@sbaranga-arm,@stephenhines,@TNorthover | ## Extended Description The AArch64 backend runs into a fatal error with the -mgeneral-regs-only when inline assembly refers to a SIMD register. A reduced test-case: $ cat > clobber.c << EOF void dummy() { __asm__ volatile ("" ::: "v0"); } EOF $ clang -c clobber.c -target aarch64-linux-gnu -mgeneral-regs-only fatal error: error in backend: Do not know how to split the result of this operator! The code is compiled by gcc, though. Seems like gcc is lenient with respect to this flag. From https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html: -mgeneral-regs-only Generate code which uses only the general-purpose registers. This will prevent the compiler from using floating-point and Advanced SIMD registers but will not impose any restrictions on the assembler. From http://clang.llvm.org/docs/UsersManual.html: -mgeneral-regs-only Generate code which only uses the general purpose registers. This option restricts the generated code to use general registers only. This only applies to the AArch64 architecture. There are two possible actions here: 1. Match gcc and allow inline assembly to have SIMD registers. This may be hard to do, considering that '-mgeneral-regs-only' just passes '-targe t-feature -fp-armv8 -target-feature -crypto -target-feature -neon' to the driver. 2. Make the driver not crash, and issue an error instead. This usage seems prevalent in the kernel, which uses -mgeneral-regs-only to avoid saving and restoring the userspace FPSIMD context on every syscall, but hard-codes FPSIMD or crypto instructions in the handful of places they're useful.