-
Notifications
You must be signed in to change notification settings - Fork 950
[otbn] Add OTBN simulator implementation of SIMD instructions #29232
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
Conversation
b011e7b to
5833a37
Compare
2366588 to
4aa8f12
Compare
andrea-caforio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been using this code for months at this point without problems.
4aa8f12 to
ca48c13
Compare
rswarbrick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial notes (I got as far as BNMULV).
It looks really nice though!
98e6442 to
d2fd698
Compare
rswarbrick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some notes here about the rest of the "Add vectorized bignum instructions" commit.
Lots of nitty notes (sorry), but I really like this! My inner functional programmer wishes that we could write things as maps over lists of lane elements that get glued together at the end, but that's a dream for a follow-up, I think :-D
rswarbrick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a couple of notes about the penultimate commit.
For the last one, are you sure we actually need to check the files in? Rather than checking in a load of auto-generated text, I think it would be much better to check the testbench so that it can generate them on the fly.
ee3d5e0 to
bdc2c64
Compare
|
@rswarbrick Thanks for the review! I adapted the points and also extended the simulator test framework that it now autogenerates most of the SIMD tests. The remaining non-generated tests are not worth the effort to write a script to auto-generate them. |
d3e52b7 to
d17596c
Compare
rswarbrick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great! Thanks!
nasahlpa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the implementation & addressing all feedback!
Before merging this, we should first merge #29231 and rebase this PR afterwards.
de7f0f7 to
545e76e
Compare
This adds the vectorized instructions to the OTBN simulator. Signed-off-by: Pascal Etterli <[email protected]>
…r test cases The vectorized instruction simulator implementation is tested against simple test programs. This adds a script which generates such programs instead of writing them by hand. Signed-off-by: Pascal Etterli <[email protected]>
This commit adds simple tests to test the simulator implementation of the vectorized instructions. Most of these tests are autogenerated when running a test using the generate_bn_simd_tests.py scripts. Signed-off-by: Pascal Etterli <[email protected]>
545e76e to
f9356fc
Compare
This PR is based upon #29231 (first 3 commits) and adds the simulator implementation for the new vectorized instructions.
Note, merging this PR will not yet resolve the issue that DV tests fail due to illegal instructions. For this the actual RTL implementation is also required.