riscv-bitmanip
riscv-bitmanip copied to clipboard
riscv
preprocessor macro for bitmanip
The gcc patch should define a preprocessor macro so end users can check to see if bitmanip support is enabled for the target. I would suggest __riscv_bitmanip since the rest of the code seems to be using bitmanip consistently.
See also the riscv/riscv-c-api-doc repo where we are documenting C API issues like preprocessor macros. I'm filing a pull request there to suggest __riscv_bitmanip which can be changed if someone has a better suggestion.
I'm filing a pull request there to suggest __riscv_bitmanip which can be changed if someone has a better suggestion.
:+1:
In the long run we'll need macros for the individual Z-extensions as well.
@mablinov added OPTION_MASK_BITMANIP_Z* to gcc in #38. I think all that's missing now is exposing those via pre-defined preprocessor macros.
Maybe instead of __riscv_bitmanip we want __riscv_ext_<extension>? For example, __riscv_ext_m, __riscv_ext_c, __riscv_ext_ztso, __riscv_ext_b, or __riscv_ext_zbb.
The compiler already has support for __riscv_compressed, __riscv_atomic, __riscv_mul, etc. Some of these are already in use by real code. I know that __riscv_compressed is in use, because we got a bug report when we accidentally broke it. So we can't drop support for them.
We could add a parallel scheme where we also have __riscv_ext_X for all of the extensions. This does have the advantage that it is easier to guess the appropriate preprocessor macro name for an extension, and it extends better to large number of extensions. The old scheme might be better when non-RISC-V experts need to modify RISC-V code, as it is clearer what the macros mean, but that isn't a major scheme. I'd be OK with adding another set of macros like this.
This should preferably be discussed somewhere where LLVM developers are also listening, as gcc folks should not unilaterally make decisions like this. We have been maintaining a doc at https://github.com/riscv/riscv-c-api-doc for C level issues like this. sw-dev also works.