riscv_vector.h intrinsics should be target-gated, not preprocessor-gated

[malaterre]

This is a complete duplicate of issue #56480 but for riscv-64.

Clang's intrinsics headers on Riscv-64 contain code like:

% head -20 /usr/lib/llvm-14/lib/clang/14.0.6/include/riscv_vector.h | tail -5

#ifndef __riscv_vector
#error "Vector intrinsics require the vector extension."
#endif

This means that one can only use Riscv-64 intrinsics in TUs that mark the feature as available for the entire intrinsic, e.g. via -march flags. In contrast, the x86 intrinsics are consistently defined, but tagged with __attribute__((__target__("whatever"))):

• https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/avx2intrin.h#L18

See also:

• https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=ae5e29239e28818f807cf11775c95c4243d9a256;hp=b8c7c62b2dbbdf355adb56d8250e68222ae0febb

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

[kito-cheng]

RISC-V didn't have define formal spec for __attribute__((__target__("whatever"))) yet, but isn't highly dependent on this I think, current I am trying to improve the intrinsic header by pragma, and that could be resolve easier once this landing.

[jan-wassenberg]

I'll be happy to update Highway for this once it lands. (This will enable runtime dispatch, i.e. only using V when it is supported, which allows deployment to any CPU without having to assume that it support V.)

[jan-wassenberg]

Any updates on this? We are seeing build errors. It seems our internal riscv_vector.h lacks the

#ifndef __riscv_vector
#error "Vector intrinsics require the vector extension."
#endif

, which is good, whereas clang 16-18 headers still appear to have it. Would be great if the #error could be removed.

The previously mentioned "Lazily add RVV C intrinsics." has indeed landed :)

[dzaima]

13b653a#diff-cad18d641a356be425d3530df6de7522de416ec8c08aed9ed751aa3e182e5e0a (clang ≥19) removed the #error, which should mean this can be closed?

[jan-wassenberg]

Thanks for the heads-up. We're adding the required target attributes in the above change, would be good to verify it works in Godbolt before we consider this closed.
Godbolt usually updates overnight, so I'll check back tomorrow.