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.

[dzaima]

Oh, looks like that the e64 & e8mf8/e16mf4/e32mf2 setvl/setvlmax intrinsics are still preprocessor-gated: https://godbolt.org/z/a6o8onbzs

Works with just manually copy-pasting them from the header, with slightly weird but understandable error messages where applicable: https://godbolt.org/z/rrvx6rof8

llvm-project/clang/include/clang/Basic/riscv_vector.td

Lines 633 to 671 in f343fee

	#if __riscv_v_elen >= 64
	#define __riscv_vsetvl_e8mf8(avl) __builtin_rvv_vsetvli((size_t)(avl), 0, 5)
	#define __riscv_vsetvl_e16mf4(avl) __builtin_rvv_vsetvli((size_t)(avl), 1, 6)
	#define __riscv_vsetvl_e32mf2(avl) __builtin_rvv_vsetvli((size_t)(avl), 2, 7)
	#define __riscv_vsetvl_e64m1(avl) __builtin_rvv_vsetvli((size_t)(avl), 3, 0)
	#define __riscv_vsetvl_e64m2(avl) __builtin_rvv_vsetvli((size_t)(avl), 3, 1)
	#define __riscv_vsetvl_e64m4(avl) __builtin_rvv_vsetvli((size_t)(avl), 3, 2)
	#define __riscv_vsetvl_e64m8(avl) __builtin_rvv_vsetvli((size_t)(avl), 3, 3)
	#endif
	#define __riscv_vsetvlmax_e8mf4() __builtin_rvv_vsetvlimax(0, 6)
	#define __riscv_vsetvlmax_e8mf2() __builtin_rvv_vsetvlimax(0, 7)
	#define __riscv_vsetvlmax_e8m1() __builtin_rvv_vsetvlimax(0, 0)
	#define __riscv_vsetvlmax_e8m2() __builtin_rvv_vsetvlimax(0, 1)
	#define __riscv_vsetvlmax_e8m4() __builtin_rvv_vsetvlimax(0, 2)
	#define __riscv_vsetvlmax_e8m8() __builtin_rvv_vsetvlimax(0, 3)
	#define __riscv_vsetvlmax_e16mf2() __builtin_rvv_vsetvlimax(1, 7)
	#define __riscv_vsetvlmax_e16m1() __builtin_rvv_vsetvlimax(1, 0)
	#define __riscv_vsetvlmax_e16m2() __builtin_rvv_vsetvlimax(1, 1)
	#define __riscv_vsetvlmax_e16m4() __builtin_rvv_vsetvlimax(1, 2)
	#define __riscv_vsetvlmax_e16m8() __builtin_rvv_vsetvlimax(1, 3)
	#define __riscv_vsetvlmax_e32m1() __builtin_rvv_vsetvlimax(2, 0)
	#define __riscv_vsetvlmax_e32m2() __builtin_rvv_vsetvlimax(2, 1)
	#define __riscv_vsetvlmax_e32m4() __builtin_rvv_vsetvlimax(2, 2)
	#define __riscv_vsetvlmax_e32m8() __builtin_rvv_vsetvlimax(2, 3)
	#if __riscv_v_elen >= 64
	#define __riscv_vsetvlmax_e8mf8() __builtin_rvv_vsetvlimax(0, 5)
	#define __riscv_vsetvlmax_e16mf4() __builtin_rvv_vsetvlimax(1, 6)
	#define __riscv_vsetvlmax_e32mf2() __builtin_rvv_vsetvlimax(2, 7)
	#define __riscv_vsetvlmax_e64m1() __builtin_rvv_vsetvlimax(3, 0)
	#define __riscv_vsetvlmax_e64m2() __builtin_rvv_vsetvlimax(3, 1)
	#define __riscv_vsetvlmax_e64m4() __builtin_rvv_vsetvlimax(3, 2)
	#define __riscv_vsetvlmax_e64m8() __builtin_rvv_vsetvlimax(3, 3)
	#endif

[dzaima]

Additionally, __riscv_v_intrinsic is also only enabled if vector is globally enabled: https://godbolt.org/z/ndTf1YM6M; gcc falls into that trap too. (unlike __riscv_v_elen & co, __riscv_v_intrinsic should always have the same value so should be safe globally)

[jan-wassenberg]

Bummer, thanks for checking :/ Do you know of anyone working on/towards this?

[dzaima]

Haven't noticed anything (but I'm just browsing random issues every now and then); @topperc / @4vtomat?