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

[RFC] Add int_xlen_t and uint_xlen_t to psabi #158

Closed
kito-cheng opened this issue Jul 17, 2020 · 17 comments
Closed

[RFC] Add int_xlen_t and uint_xlen_t to psabi #158

kito-cheng opened this issue Jul 17, 2020 · 17 comments

Comments

@kito-cheng
Copy link
Collaborator

I saw there is similar request/needs for XLEN-width integer type in several intrinsic function design, including V-extension, P-extension and B-extension, current solution is using long and unsigned long as XLEN-width integer type.

It's work correctly, but it might not work if we have ilp32 for RV64, long could be defined as 32-bits.

So I think we could propose add int_xlen_t and uint_xlen_t to RISC-V standard type, however there will be a transition period, some compiler support, and some compiler not support that, add a macro like __riscv_xlen_int might help that.

New Types

Type name Size Alignment
RV32 RV64 RV32 RV64
int_xlen_t 4 8 4 8
uint_xlen_t 4 8 4 8

New Predefined Macro

  • __riscv_xlen_int will defined if compiler has provide int_xlen_t and uint_xlen_t.
@kito-cheng
Copy link
Collaborator Author

@jim-wilson @knightsifive @asb @lenary @clairexen

@aswaterman
Copy link
Contributor

Agreed, we want to avoid proliferation of the assumption that unsigned long, size_t, uintptr_t, etc. are XLEN-sized, in anticipation of a future RV64 ILP32 ABI.

@nick-knight
Copy link
Contributor

I'm certainly opposed to using long to mean "XLEN-width integer": indeed, I usually leverage the GCC and Clang/LLVM macro __riscv_xlen to define these 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

But when writing C code to be portable across RV32 and RV64, I've found that I need XLEN in other places than just variable declarations, e.g., when bit-twiddling. So I'm wondering if it might be simpler just to standardize __riscv_xlen and let the user define these types as they need.

Along this same line of thought, other macros, like __riscv_flen and ones indicating presence/absence of various ISA extensions, might also be useful. I don't know how much belongs in the SysV psABI, however.

@aswaterman
Copy link
Contributor

aswaterman commented Jul 17, 2020

Both of those macros are already part of the toolchain conventions doc (as are related macros that describe the calling convention): https://github.com/riscv/riscv-toolchain-conventions#cc-preprocessor-definitions

It's possible that this document is a more appropriate place for such things, since they really are part of the RISC-V C API, not merely "conventions".

@jim-wilson
Copy link
Collaborator

We also have https://github.com/riscv/riscv-c-api-doc/blob/master/riscv-c-api.md that defines command line options, predefined macros, etc. Looks like a lot of overlap between riscv-toolchain-conventions and riscv-c-api-doc.

@aswaterman
Copy link
Contributor

"C API" sounds like the right place for this stuff. I guess at some point we should merge the toolchain conventions document into the C API doc and deprecate the toolchain conventions repo.

@PkmX
Copy link
Contributor

PkmX commented Jul 17, 2020

I don't think the compiler can introduce these types without leading underscores since they may conflict with user code. It may be easier to standardize on a <riscv.h> header that provides these typedefs (and maybe other types/functions/macros in the future).

@lenary
Copy link
Contributor

lenary commented Jul 17, 2020

I'm not sure why long isn't good enough right now, but I don't want to stop this going ahead if people are adamant we will get an ILP32 ABI for RV64.

Does it make sense to define a __riscv_flen_t at the same time, which is the architecture's (not the ABI's) float type? I don't know what this would be when there's no floating-point support in the architecture.

@PkmX is right about needing a prefix for non-standard built-in types. It does make sense to have a header, and that's something we need to work out as we work out what we're doing about other intrinsic headers like <rvintrin.h> which I think the B and V extensions use.

@aswaterman I'm not convinced we should deprecate the toolchain conventions repo - a lot of those conventions extend beyond C to assemblers (in terms of things like -march arguments), and also to other compilers like Rust etc. It would be good to do some merging, though, I agree on that point.

@kito-cheng
Copy link
Collaborator Author

Does it make sense to define a __riscv_flen_t at the same time, which is the architecture's (not the ABI's) float type? I don't know what this would be when there's no floating-point support in the architecture.

I also consider about the int_flen_t and uint_flen_t before, but I am not sure the use case for that, so I would like to introduce XLEN-width integer type this time only, let me know if you found

@PkmX Thanks you point out this, introducing new types by header file sounds good idea.

@sorear
Copy link
Collaborator

sorear commented Jul 21, 2020

We already have rvintrin.h though?

@MaskRay
Copy link
Collaborator

MaskRay commented Jul 21, 2020

psABI only defines widths/alignments of fundamental types. https://github.com/riscv/riscv-c-api-doc may be a good place. POSIX reserves _t suffix....... though there is precedent like ARM NEON int8x4_t.

@sorear
Copy link
Collaborator

sorear commented Jul 22, 2020

_t is reserved for the implementation, which includes arch-specific compilers, so we're fine to use it as long as we're not conflicting with a compiler or libc that wants riscv support?

@kito-cheng
Copy link
Collaborator Author

@sorear rvintrin.h[1] isn't included in standard RISC-V toolchain yet, it only appear in B-extension toolchian, but it's one option to put those type there.

[1] https://github.com/riscv/riscv-gcc/blob/riscv-bitmanip/gcc/config/riscv/rvintrin.h

@MaskRay Yeah, I agree https://github.com/riscv/riscv-c-api-doc is a good place :)

@HanKuanChen
Copy link

I would think this type is not about intrinsic. Define it in a <riscv.h> just like @PkmX mentioned is a good idea.

In addition, maybe it should be named like xlen_t and uxlen_t instead of int_xlen_t and uint_xlen_t. "xlen" is already very clear about the usage. riscv will not have something like float_xlen_t or char_xlen_t. If users want a floating version, we can have a flen_t.

About whether it should have a __ prefix, I personally prefer not have. Introduce new types always make a potential naming conflict problem. Intel has __m64 and __m128i, while Arm has poly64_t and svint8_t. Both of the naming style has followers. However, v extension has already use a naming style without underscores. Make consistency is more important. So I would suggest xlen_t instead of __xlen_t.

Finally, I usually see these types with _t. If a type is named xlen, it is probably a class/struct/union. xlen_t is more clear than xlen.

@nick-knight
Copy link
Contributor

I'd avoid using the term "UXLEN", which also means the "effective XLEN in U-mode" (defined in Vol. II, Sec. 3.1.6.2).

@ebahapo
Copy link
Contributor

ebahapo commented Dec 9, 2020

Does it make sense to define a __riscv_flen_t at the same time, which is the architecture's (not the ABI's) float type? I don't know what this would be when there's no floating-point support in the architecture.

I don't think that there's an architecture float type. Surely, with the F extension it's float, but which one of float or double would be for the D extension?

@kito-cheng
Copy link
Collaborator Author

Hi everyone:

Thanks all your inputs, I create an PR on riscv-c-api-doc, and I gonna close this issue, any further comment are welcome in that PR.

[1] riscv-non-isa/riscv-c-api-doc#14

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

No branches or pull requests

10 participants