riscv-non-isa / rvv-intrinsic-doc

https://jira.riscv.org/browse/RVG-153
BSD 3-Clause "New" or "Revised" License
294 stars 89 forks source link

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 #376

Open camel-cdr opened 3 weeks ago

camel-cdr commented 3 weeks ago

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 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 commented 3 weeks ago

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.