attribute target syntax doesn't match -march options

[nickdesaulniers]

Bugzilla Link	43448
Version	trunk
OS	Linux
Blocks	#4440
CC	@DougGregor,@echristo,@erichkeane,@fhahn,@kbeyls,@zygoloid,@stephenhines

Extended Description

When looking into an ISA-extension-assembler-directive related issue in assembling the Linux kernel w/ Clang (ClangBuiltLinux/linux#671, ClangBuiltLinux/linux#573), I ran into some issue with the ARMv8.1 lse extensions.

It seems that we can either:

1. use an assembler .arch armv8-a+lse directive
2. use a command line arg -march=armv8-a+lse
3. use a function level attribute

Unfortunately, 3 gets a little complicated in terms of our "compatibility w/ GCC" which is a constraint for compiling the Linux kernel w/ Clang.

__attribute__((target("arch=armv8-a+lse")))
void foo (void){}

produces a warning in Clang, but is accepted by GCC.

__attribute__((target("lse")))
void foo (void){}

produces an error in GCC, but is accepted by Clang.

I find the current behavior in Clang for 3 inconsistent with 1+2.

[erichkeane]

For 'target', the list of "arch=" is TargetInfo.isValidCPUName, which AArch64 is looking up via llvm::AArch64::parseCPUArch. This list comes from AArch64TargetParser.def, however the AARCH64_CPU_NAME macro is used.

Interestingly, the march handling ends up using parseArch (Splitting on the +).

The + syntax isn't supported by 'target'. It isn't supported by GCC either (as you can see by doing something in x86-64 mode like atom+avx2: https://godbolt.org/z/WoDnbM

As you can see, GCC is treating that as a single option. If you give garbage to the AArch64 version however, you get a list: https://godbolt.org/z/0PM6Cc
armv8-a armv8.1-a armv8.2-a armv8.3-a armv8.4-a

Note that the +lse isn't included in these, which makes me think their AArch64 handler is working a touch differently and dealing with the +lse itself.

We likely needs to alter the way isValidCPUName works for that target to properly give the right name/arch.

I'll note that isValidCPUName is used exclusively for 'target' support, so it ought to be safe to do.

Additionally, we may want to change the way the TargetAttr parser works in ARM cases to support "+" as a separator.

[erichkeane]

This ends up being about a 75% solution here: https://reviews.llvm.org/D68050

I don't know what LLVM changes need to happen however.

[nickdesaulniers]

FWIW, this is the documentation the ARM64 kernel maintainer pointed me to: https://sourceware.org/binutils/docs/as/AArch64-Extensions.html#AArch64-Extensions

[fhahn]

FWIW, this is the documentation the ARM64 kernel maintainer pointed me to:
https://sourceware.org/binutils/docs/as/AArch64-Extensions.html#AArch64-
Extensions

That's interesting, thanks! So to remove an extension in GCC, you seem to have to prepend no, so you'd use +nolse.

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Nick Desaulniers (paternity leave) (nickdesaulniers)

| | | | --- | --- | | Bugzilla Link | [43448](https://llvm.org/bz43448) | | Version | trunk | | OS | Linux | | Blocks | llvm/llvm-project#4440 | | CC | @DougGregor,@echristo,@erichkeane,@fhahn,@kbeyls,@zygoloid,@stephenhines |

Extended Description

When looking into an ISA-extension-assembler-directive related issue in assembling the Linux kernel w/ Clang (ClangBuiltLinux/linux#671, ClangBuiltLinux/linux#573), I ran into some issue with the ARMv8.1 lse extensions.

It seems that we can either:

1. use an assembler .arch armv8-a+lse directive
2. use a command line arg -march=armv8-a+lse
3. use a function level attribute

Unfortunately, 3 gets a little complicated in terms of our "compatibility w/ GCC" which is a constraint for compiling the Linux kernel w/ Clang.

__attribute__((target("arch=armv8-a+lse")))
void foo (void){}

produces a warning in Clang, but is accepted by GCC.

__attribute__((target("lse")))
void foo (void){}

produces an error in GCC, but is accepted by Clang.

I find the current behavior in Clang for 3 inconsistent with 1+2.

[davemgreen]

We implemented mostly the same target attribute support as GCC a while ago.
https://godbolt.org/z/nf66T61o7
There are still some changes happening to support all the combinations, but it should work as described in this ticket now.