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

Bitfield syntax updates #373

Merged
merged 2 commits into from
Jan 31, 2024
Merged

Conversation

Alasdair
Copy link
Collaborator

Sail for a while now (at least several versions at this point) has supported bitfield syntax that is closer to regular bit vector syntax. This PR updates the model to use this newer syntax. In theory this might be even more efficient (although I have not benchmarked), as the old syntax used special accessor functions that are generated by the bitfield declaration, whereas the new syntax avoids that indirection.

To summarise, for a bitfield:

bitfield B : bits(32) = {
  F : 7 .. 0
}
  • Accessing can be x[F] rather than x.F().
  • Similarly, modifying can be x[F] = 0x00 rather than x->F() = 0x00.
  • Updating a field can be done as [x with F = 0x00] rather than update_F(x, 0x00).
  • Getting all the bits is now x.bits rather than x.bits(), and the same for setting with x.bits = 0x0000_0000.

Alternatively, one can look at the manual section on bitfields

The commit adding the update syntax is separate, as it was slightly more manual than the other changes. In general, code that looked like:

let x = update_F1(x, y);
let x = update_F2(x, z);
x

can be changed to

[x with F1 = y, F2 = z]

I avoided making this update change in code I know is going to change soon (like the virtual memory code), or the PMP definitions, and I have no plans to remove the update_ functions anyway.

Use newer bitfield syntax, which has been part of Sail for
a while now. Should in theory be more efficient as it removes
a level of indirection for bitfield accesses.

It's also much more friendly to `sail -fmt`, which has no idea
how to handle the old bitfield syntax.
Bitfields allow [<bitfield> with field = value], like bitvectors,
so use that instead of the old-style `update_field` overloads.
Copy link
Collaborator

@arichardson arichardson left a comment

Choose a reason for hiding this comment

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

Was not aware of that syntax. I think this new version looks better.

Copy link

Test Results

712 tests  ±0   712 ✔️ ±0   0s ⏱️ ±0s
    6 suites ±0       0 💤 ±0 
    1 files   ±0       0 ±0 

Results for commit e3fd9bb. ± Comparison against base commit 330e706.

@Alasdair
Copy link
Collaborator Author

It's been around for a while but I don't think I had documented it well enough. It's in my new asciidoc manual now at least as the recommended way.

I might make the x.F() syntax and x->F() syntax a warning in the next version of Sail. If I ever add namespacing I think I'd want to use . as the separator, and having an <expression> . <id> () construct in the grammar will make disambiguating that annoying.

@Timmmm
Copy link
Collaborator

Timmmm commented Jan 15, 2024

@billmcspadden-riscv I think this should be merged ASAP to avoid inflicting painful rebasing on Alasdair.

@arichardson
Copy link
Collaborator

This is a purely syntactic change and has been open for 1.5 months now. Can we merge it?

@billmcspadden-riscv billmcspadden-riscv merged commit e7c369d into riscv:master Jan 31, 2024
2 checks passed
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 this pull request may close these issues.

4 participants