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.

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

Should vcpop.m return type be size_t? #166

Closed
HanKuanChen opened this issue Sep 15, 2022 · 2 comments · Fixed by #287
Closed

Should vcpop.m return type be size_t? #166

HanKuanChen opened this issue Sep 15, 2022 · 2 comments · Fixed by #287

Comments

@HanKuanChen
Copy link
Contributor

Current interface

unsigned long vcpop_m_b1 (vbool1_t op1, size_t vl);

However, the semantic of vcpop.m is

The vcpop.m instruction counts the number of mask elements of the active elements of the vector source mask register that have the value 1 and writes the result to a scalar x register.

That means the maximum value depends on VL. And VL related interface is

size_t vsetvl_e8mf8 (size_t avl);
size_t vsetvlmax_e8mf8 ();

So, should we change the interface to

size_t vcpop_m_b1 (vbool1_t op1, size_t vl);
@jan-wassenberg
Copy link

Sounds good to me.

@nick-knight
Copy link
Collaborator

I've always disagreed with using implementation-defined size_t for VL, and would prefer something like riscv-non-isa/riscv-c-api-doc#14 that more closely matches the architecture: VL is defined as an XLEN-bit CSR and its value is always exchanged through (XLEN-bit) X-registers.

My disagreement, however, is very mild: C99 requires size_t be at least 16 bits, and of course is XLEN bits in our standard ABIs.

I have a related complaint for vfirst.m. Currently we use long, which again is implementation defined but the C language guarantees it has sufficient range for our purposes, and also it is XLEN bits in our standard ABIs.

So I support the proposal here, and support maintaining the status quo for vfirst.m, but still think the XLEN-width types provide a better solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants