riscv-non-isa/riscv-toolchain-conventions

C Compiler Builtins for CSR Accesses

lenary opened this issue · 3 comments

Ages ago, I proposed a Clang patch to add builtins for CSR accesses, so that there would be less reliance on inline assembly. I intended to finish that patch, and then make a proposal here, but I have not managed to do so.

The patch is here: https://reviews.llvm.org/D71778

It proposes:

long __builtin_riscv_csr_read_write(long, long);
long __builtin_riscv_csr_read_set(long, long);
long __builtin_riscv_csr_read_clear(long, long);

Which correspond to csrrw, csrrs and csrrc respectively (and took a CSR ID as the first argument, which had to be an immediate).

Is this useful? What do people think? I think I was also going to propose nicer names for these in rvintrin.h, maybe including equivalents for the pseudo-instructions, but I don't recall all of my intentions from back then any more.

Instead of long, shouldn't you use uint_xlen_t (v. riscv-non-isa/riscv-c-api-doc#14) instead?

IIRC we've discuss that once on LLVM sync up meeting, there is concern for those intrinsic, the code might compile-able when optimization enabled (e.g. compile with -O1 or higher) but not compile-able when optimization disabled, because the CSR number must be integer literal.

For example, following code is OK if you compile with -O1, but compiler will complain if you compile with -O0.

void foo(){
    int x = 1;
    asm ("%0" :: "i"(x));
}

Those CSR intrinsic having similar issue as the above example, but there is svget* and svset* in ACLE for SVE, which can emit error when the input isn't constant, so I think it should not be a problem now.

Example from ACLE:

#include <arm_sve.h>

void foo(){
   svint32x2_t ta;
   int idx = 1;
   svint32_t a = svget2 (ta, idx);
}

Diagnostics message from clang and GCC.

$ aarch64-elf-gcc sve.svget.c  -march=armv8.2-a+sve -O
sve.svget.c: In function 'foo':
sve.svget.c:6:18: error: argument 2 of 'svget2' must be an integer constant expression
    6 |    svint32_t a = svget2 (ta, idx);
      |                  ^~~~~~
$ clang -target aarch64-elf sve.c  -o  - -march=armv8.2-a+sve -S 
sve.c:8:18: error: argument to 'svget2' must be a constant integer
   svint32_t a = svget2 (ta, idx);
                 ^           ~~~
1 error generated.

So basically I am OK with those CSR intrinsic now, but just remind that we need to carefully to implement and test to make sure the argument must be constant integer.

Yeah I recall something like this. I would support the aliased versions being implemented as macros, rather than functions, if that suggestion is any help.