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.

Sign up for GitHub

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

Jump to bottom

Make methods of mem_t virtual to allow overriding #1415

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Make methods of mem_t virtual to allow overriding #1415

merged 1 commit into from
Jul 20, 2023

Conversation

michalt
Copy link
Contributor

@michalt michalt commented Jul 14, 2023

This allows the users to provide their own custom implementations of mem_t when constructing sim_t.

Fixes #1408.

@michalt
Copy link
Contributor Author

michalt commented Jul 14, 2023

@aswaterman @jerryz123 I know this wasn't your preferred option when discussing this on #1408, but looking at the code now, I think it'd be a bit nicer if we separated the interface from the implementation: We could have anabstract_mem_t just for the interface, and a mem_t (or a sparse_mem_t) that implements the interface; sim_t would take abstract_mem_t to allow injecting custom implementations. That being said, I don't feel strongly, and the current version of the PR does solve the problem for my team. 😃

@aswaterman
Copy link
Collaborator

I'm OK with that alternative if @jerryz123 is.

@jerryz123
Copy link
Collaborator

I have no problems with that.

@michalt
Copy link
Contributor Author

michalt commented Jul 17, 2023

Great, I updated the PR with the changes. Let me know what you think! Thanks! 😄

@aswaterman
Copy link
Collaborator

Seems like there are some CI failures.

@jerryz123
Copy link
Collaborator

Can you squash the two commits together? Each commit in PRs to this project is checked for functionality.

@michalt
Copy link
Contributor Author

michalt commented Jul 20, 2023

@jerryz123 Done! 😃 (BTW is there a way for me to trigger the tests?)

@michalt
Introduce abstract_mem_t to allow custom implementations
cb01351 
This change allows to create custom implementations of `abstract_mem_t`
and inject them when constructing `sim_t`. The current `mem_t`
implementation remains unchanged.

Fixes riscv-software-src#1408.
@michalt
Copy link
Contributor Author

michalt commented Jul 20, 2023

FYI: I missed one mem_t and had to push again.

@jerryz123 jerryz123 merged commit 1b41ed3 into riscv-software-src:master Jul 20, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottj97 scottj97 scottj97 approved these changes

@jerryz123 jerryz123 jerryz123 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow overriding mem_t
4 participants
@michalt @aswaterman @jerryz123 @scottj97