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

[pirama-arumuga-nainar]

Bugzilla Link	30792
Version	trunk
OS	Linux
Blocks	#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.

[pirama-arumuga-nainar]

assigned to @gburgessiv

[sbaranga-arm]

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]

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.

[sbaranga-arm]

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, 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.

[sbaranga-arm]

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

[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).

[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.

[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.

[sbaranga-arm]

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

[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

[m-gupta]

George, Are you working on it?

[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.

[gburgessiv]

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

[edwintorok]

mentioned in issue #4440