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

Type ambiguities/incosistencies #95

Open
bjc opened this issue Mar 2, 2022 · 4 comments
Open

Type ambiguities/incosistencies #95

bjc opened this issue Mar 2, 2022 · 4 comments

Comments

@bjc
Copy link

bjc commented Mar 2, 2022

The current SBI specification assumes familiarity with C/C++ type widths, which I would argue is inappropriate for this level of documentation. I would like to see the type widths in use formally specified, ie:

int : 16 bits
long: 32 bits
long long: 64 bits

In particular, int as used in the legacy console functions could be almost anything. There are also a number of functions where the explicitly-sized types are used as well as the implicity-sized ones:

For uint64_t:

  • both instances of sbi_set_timer,
  • sbi_pmu_counter_config_matching,
  • sbi_pmu_counter_start

For uint32_t:

  • sbi_hart_suspend
  • sbi_system_reset

The uint32_t are particularly confusing, as otherwise the document implies pretty strongly that long is 32 bits wide, and the examples above use both long and uint32_t. If there's some rationale for the use of both types, I didn't see it in the spec.

I believe the appropriate thing to do is:

  • use the explicitly-sized types everwhere (ie, uint64_t etc), and
  • define, at the beginning of the specification, what size those types are, or at least provide a reference to their definition elsewhere.
@aswaterman
Copy link

I would prefer just documenting the assumption that this is an LP64 ABI for RV64 and ILP32 ABI for RV32. That removes the definitional ambiguity without requiring significant changes.

The problem with using explicitly-sized types is that they don't do the right thing when the size is supposed to change between RV64 and RV32. By contrast, in the standard RISC-V ABIs, long has the desired property.

@bjc
Copy link
Author

bjc commented Mar 2, 2022

MXLEN is used in other documentation to specify the register width, if that's how long is being used now. My issue is that we're not all C programmers - I'm implementing an SBI and SEE in Rust, which has no concept of register width. It does know how big a pointer is, but those are separate concerns.

Additionally, MXLEN is run-time variable, as opposed to long which will break if the size of MXLEN changes. Let alone how it works when M-mode has a different MXLEN than S/U-Mode.

I don't believe the changes are particularly significant, and should have no effect on existing, conformant, programs. If they do have an effect, then that's either a specification oversight or a program bug. Being explicit is highly valuable in the context of a specification that defines foundational calling conventions, which this specification does.

@repnop
Copy link

repnop commented Mar 6, 2022

I think if its desired to keep C semantics, defining some reg_t type and specifying that it occupies the same bitwidth as a register or is defined in terms of XLEN would be a good compromise. Basically just anything that makes it much more explicit what the actual sizes are, because sometimes C integer types don't even agree on the same size on the same architecture: Windows specifies that a long is 4-bytes regardless of x86 or x86_64, whereas on x86_64 Linux systems long is 8 bytes large.

@aswaterman
Copy link

@repnop Documenting which ABI is assumed is sufficient to convey the size of the C datatypes. I still prefer that approach, but I agree defining and using an XLEN-bit type is a suitable alternative.

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

3 participants