Public Review: __riscv_v_intrinsic needs to be defined and <riscv_vector.h> available reguardless of -march to allow intrinsic usage in __attribute__((target("arch=+v"))) functions

[camel-cdr]

A very common pattern in intrinsics usage is specializing functions for a specific ISA extension and implementing runtime dispatch.
This means that you can't globally defined -march to include support for the "V" extension, buy you rather use __attribute__((target("arch=+v"))) on the functions you specifically optimize with intrinsics and runtime dispatch.

The current definitions of <riscv_vector.h> and __riscv_v_intrinsic are unclear and thus currently unusable for a lot of libraries using this paradigm.

The __riscv_v_intrinsic macro is the C macro to test the compiler’s support for the RISC-V "V"
extension intrinsics.

It's ambiguous whether this can be interpreted as __riscv_v_intrinsic should be defined when compiling for the "V" extension (V is in march), or __riscv_v_intrinsic should be defined if the compiler support the "V" intrinsics.

Both clang and gcc interpret it to mean the former, so you can't use __riscv_v_intrinsic to detect support for "V" intrinsics: https://godbolt.org/z/EMbYYrr5Y

With <riscv_vector.h> included, availability of intrinsic variants depends on the required
architecture of their corresponding vector instructions. The supported architecture is specified to the
compiler using the -march option

Again, it's unclear whether including <riscv_vector.h> is supposed to work when march doesn't include support for a subset of the vector extension.
But, both gcc and clang support including <riscv_vector.h> when it's not specified in march, such that it can be used in __attribute__((target("arch=+v"))) code: https://godbolt.org/z/oMrY4xn75

I think both definitions should make it explicit, that __riscv_v_intrinsic shall be defined and <riscv_vector.h> available regardless of -march to allow intrinsic usage in __attribute__((target("arch=+v"))) functions.

[jan-wassenberg]

Thanks for raising this. We strongly support __riscv_v_intrinsic for detecting compiler support, plus being able to include riscv_vector without -march, plus allowing target pragmas to replace -march.

These features will help RISC-V adoption by enabling binaries that work on CPUs regardless of its V support. Without them, users would have to recompile for the particular CPU, which is a non-starter in a server/cloud context.

[rofirrim]

@camel-cdr So my reading from your proposal above is that we'd like to have __riscv_v_intrinsic unconditionally enabled as long as the compiler provides support for <riscv_vector.h>. Is that right?

[camel-cdr]

Exactly, and you should be able to include <riscv_vector.h> without a gobal march setting, such that you can use the intrinsics in __attribute__((target("arch=+v"))) functions.

[4vtomat]

Exactly, and you should be able to include <riscv_vector.h> without a gobal march setting, such that you can use the intrinsics in __attribute__((target("arch=+v"))) functions.

I think we can now include riscv_vector.h without any march defined

[jan-wassenberg]

When we last checked, this was not working yet:
google/highway#2311
llvm/llvm-project#56592 (comment)

Seems dzaima's repro is still not working? https://godbolt.org/z/ndTf1YM6M

[rofirrim]

@kito-cheng @4vtomat I think there is value in having this for 1.0. Do you think we can aim at it? If not I guess we could potpone it for a later revision of the spec.

[kito-cheng]

@rofirrim

I think there is value in having this for 1.0. Do you think we can aim at it? If not I guess we could potpone it for a later revision of the spec.

Yeah, I think it worth to make this change into 1.0 :)

[4vtomat]

When we last checked, this was not working yet: google/highway#2311 llvm/llvm-project#56592 (comment)

Seems dzaima's repro is still not working? https://godbolt.org/z/ndTf1YM6M

Oh, I mean you can include <riscv_vector.h> without defining any vector related march

[jan-wassenberg]

When we last checked, this was not working yet: google/highway#2311 llvm/llvm-project#56592 (comment)
Seems dzaima's repro is still not working? https://godbolt.org/z/ndTf1YM6M

Oh, I mean you can include <riscv_vector.h> without defining any vector related march

:) Yes, though a few other things are required for the use case (runtime dispatch) to actually work :)

[camel-cdr]

@kito-cheng

With <riscv_vector.h> included, availability of intrinsic variants depends on the required architecture of their corresponding vector instructions. The supported architecture is specified to the compiler using the -march option

It might be worth adding something like "The supported architecture is specified to the compiler globally using the -march option, or the on a per-function basis using the target attribute."