Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unify intrinsic interface for read/write vCSRs #204

Closed
wants to merge 1 commit into from
Closed

Unify intrinsic interface for read/write vCSRs #204

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions rvv-intrinsic-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,19 @@ These utility functions help users extract a smaller LMUL value from a larger LM
### Read/Write URW vector CSRs

```
enum RVV_CSR {
RVV_VSTART = 0,
RVV_VXSAT,
RVV_VXRM,
RVV_VCSR,
};
unsigned long __riscv_read_csr_vstart();
void __riscv_write_csr_vstart(unsigned long value);

unsigned long vread_csr(enum RVV_CSR csr);
void vwrite_csr(enum RVV_CSR csr, unsigned long value);
unsigned long __riscv_read_csr_vxsat();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is unsigned long the right type for a XLEN-bit value?

I.e. the combination of RV64 with ILP32 could cause issues.

We probably want intX_t and uintX_t for RISC-V.

@kito-cheng: any ideas?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that should be intxlen_t would be more semantic correct here, let push this riscv-non-isa/riscv-c-api-doc#14 again next week.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple notes:

  • One doesn't need a type exactly sized as the CSR, but only long enough (e.g., intmax_t would always work).
  • Types that start with 'int' may end up being used in future standardization of inttypes.h, so we should stay out of that namespace.
  • We already have the __riscv_xlen macro. So using the preprocessor, one can always determine the actual number of bytes that a CSR is wide.
  • What about __riscv_flen? Would you want to introduce another type for 'an integer that is as wide as the longest floating point register'?

Pondering all of the above points, I would recommend against introducing such a new type — all we need is a type that is at least as wide as a CSR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi all,
I see that there is already on-going discussion in the SIG toolchain mailing list [0], please proceed there as I have responded, we plan to remove vread_csr and vwrite_csr, and the RVV type discussion should be hosted there.

[0] https://lists.riscv.org/g/sig-toolchains/message/599

void __riscv_write_csr_vxsat(unsigned long value);

unsigned long vlenb();
unsigned long __riscv_read_csr_vxrm();
void __riscv_write_csr_vxrm(unsigned long value);

unsigned long __riscv_read_csr_vcsr();
void __riscv_write_csr_vcsr(unsigned long value);

unsigned long __riscv_read_csr_vlenb();
```

## 7. Vector Loads and Stores
Expand Down