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

Make extension enum not scattered #633

Open
Timmmm opened this issue Dec 5, 2024 · 2 comments · May be fixed by #634
Open

Make extension enum not scattered #633

Timmmm opened this issue Dec 5, 2024 · 2 comments · May be fixed by #634
Labels
refactor Code clean up

Comments

@Timmmm
Copy link
Collaborator

Timmmm commented Dec 5, 2024

Currently we have a scattered extension enum that we generally add entries to in the files that implement the extension, e.g. riscv_svinval.h contains:

enum clause extension = Ext_Svinval
function clause extensionEnabled(Ext_Svinval) = sys_enable_svinval()

I've found that this causes quite annoying circular dependencies, especially with functions that depend on lots of extensions (e.g. stuff around legalizing mie, mstatus, misa, etc.).

I propose making extension not scattered, and just putting it all in a big list in one file (riscv_extensions.sail)

enum extension = {
  Ext_S,
  Ext_Svinval,
  Ext_...
}

This can then be added early on in the compile order. I don't think there are any real downsides to this and it might actually be quite nice to see them all in one place.

The extensionEnabled() clauses should still be scattered.

@jordancarlin
Copy link
Collaborator

The only downside that I see is in relation to the new module system that @Alasdair is working on in #572. This would require all of the extensions to be defined in the enum even when that extension isn't included. I don't think that is a significant issue though because the extensionEnabled clause can still be in the extension specific files. Overall seems worthwhile solely for sake of dependencies.

@Timmmm
Copy link
Collaborator Author

Timmmm commented Dec 5, 2024

Yeah I think that's fine. If Sail ever has unused code warnings it might be annoying but I guess we can just add something like Rust's #[allow(unused)].

@Timmmm Timmmm linked a pull request Dec 6, 2024 that will close this issue
@Timmmm Timmmm added the refactor Code clean up label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code clean up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants