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

Constrain TLB struct bit sizes #362

Closed
wants to merge 1 commit into from

Conversation

Timmmm
Copy link
Collaborator

@Timmmm Timmmm commented Nov 28, 2023

Constraining them to <= 64 bits generates code that doesn't use GMP and is therefore much faster. This massively increases C emulation performance.

Constraining them to <= 64 bits generates code that doesn't use GMP and is therefore much faster. This massively increases C emulation performance.
Copy link

Unit Test Results

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

Results for commit 1d8731e. ± Comparison against base commit 9f4cad6.

@Alasdair
Copy link
Collaborator

LGTM 👍

Better bitvector length monomorphisation is on my agenda for the Sail -> SystemVerilog model checking path so maybe in the future this will not be needed. That may be a way off yet, so I'm not saying that as a reason to not merge this.

@ved-rivos
Copy link
Contributor

ved-rivos commented Dec 1, 2023

@Timmmm should the asidlen be 16 and not 64

@Timmmm
Copy link
Collaborator Author

Timmmm commented Dec 4, 2023

@Timmmm should the asidlen be 16 and not 64

It wasn't really the intent of this change to impose architectural restrictions; just to make Sail generate better C types, and only 64 is necessary for that. I think I'd prefer to leave it as 64 so the intent is clear and this code can be removed if/when Sail uses "small bitvector optimisation".

@Timmmm
Copy link
Collaborator Author

Timmmm commented Dec 4, 2023

(Or more monomorphisation as Alasdair mentioned.)

@arichardson
Copy link
Collaborator

I think this is no longer needed? If so can we close the pr?

@Timmmm
Copy link
Collaborator Author

Timmmm commented May 10, 2024

Yep.

@Timmmm Timmmm closed this May 10, 2024
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