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

Use fixed-width integer type instead of long? #31

Closed
nick-knight opened this issue Jul 17, 2020 · 5 comments · Fixed by #287
Closed

Use fixed-width integer type instead of long? #31

nick-knight opened this issue Jul 17, 2020 · 5 comments · Fixed by #287
Labels
Revisit after v1.0 Features or problems we will revisit after the v1.0 release

Comments

@nick-knight
Copy link
Collaborator

nick-knight commented Jul 17, 2020

In a number of places, the V-extension intrinsics API uses long or unsigned long to represent an XLEN-bit integer (signed or unsigned, resp.). It's true that long has size XLEN in the currently defined ILP32 and LP64 ABIs
https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#-named-abis
However, note that

A future version of this specification may define an ILP32 ABI for the RV64 ISA

And one can imagine a similar scenario for RV128. Hence, I think it's preferable to future-proof the intrinsics API by using typedefs [Footnote 1]. I don't think this adds much notational overhead.

Please note that we had the same discussion for the P-extension intrinsics, and arrived at the same conclusion
https://lists.riscv.org/g/tech-p-ext/topic/73148653#26

[Footnote 1]
On GCC and Clang/LLVM, you can alias the relevant types as follows:

#include <inttypes.h>
#if __riscv_xlen == 32
typedef int32_t int_xlen_t;
typedef uint32_t uint_xlen_t;
#elif __riscv_xlen == 64
typedef int64_t int_xlen_t;
typedef uint64_t uint_xlen_t;
#endif

Those who enjoy preprocessor wizardry might prefer something like

#define create_type2(a, b, c) a ## b ## c
#define create_type1(a, b, c) create_type2(a, b, c)
typedef create_type1(int,__riscv_xlen,_t) int_xlen_t;
typedef create_type1(uint,__riscv_xlen,_t) uint_xlen_t;
@kito-cheng
Copy link
Collaborator

kito-cheng commented Jul 17, 2020

I saw there is similar request in several intrinsic design, including P-extension and B-extension, maybe we should add that into psabi?

@zakk0610
Copy link
Collaborator

zakk0610 commented Aug 3, 2020

FYI: this discussion was moved to riscv-non-isa/riscv-elf-psabi-doc#158.

@topperc
Copy link
Collaborator

topperc commented Dec 8, 2020

We're working on upstreaming the rvv intrinsics to llvm/clang. I'm trying to figure out if this is an issue we should worry about for that.

I need to define builtins for vsetvl and all the intrinsics that take a vl operand. clang's system for specifying builtins mostly uses fixed types. There are currently a few special cases, for example saying a builtin needs a 64 bit type where will choose long or long long depending on which is really 64 bits. There's also a special case to say a builtin takes size_t. There isn't any target specific overrides into that code today so I don't have a way of saying a builtin takes an xlen integer and I'm not sure how to abstract it into target independent code. Maybe I can have it find the same type as attribute ((mode (word)))?

Another option might be to just say that vl is a 32-bit "unsigned" in the C interface? Greater than 4 billion element vectors seems like a lot.

@nick-knight
Copy link
Collaborator Author

nick-knight commented Dec 8, 2020

My advice is the following:

  1. Update the RVV Intrinsics spec to use uint_xlen_t, plus some explanation of what this means (see above). I.e., tell the user that the vl argument should be an XLEN-width unsigned integer, and it's up to them to correctly instantiate values of this type.

  2. In the current LLVM implementation, just use unsigned long, which will do the right thing for all extant ABIs. We can repay this technical debt in the future: once the RISC-V C API adds support for uint_xlen_t --- which I'm confident will happen --- this will have to be added to LLVM.

@eopXD
Copy link
Collaborator

eopXD commented Jul 29, 2022

We have existing proposal in riscv-c-api-doc #14 that is still pending. I understand the fastest way here is to define the types in the RVV intrinsic header, but the formal completion should allow multiple extensions to use this defined type.

This issue will be held and resolved after v1.0 since it is depending on more consensus in the RISC-V community.

@eopXD eopXD added the Revisit after v1.0 Features or problems we will revisit after the v1.0 release label Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Revisit after v1.0 Features or problems we will revisit after the v1.0 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants