-
Notifications
You must be signed in to change notification settings - Fork 173
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
Q extension #445
base: master
Are you sure you want to change the base?
Q extension #445
Conversation
24150d7
to
df7ae02
Compare
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.
Haven't read it all but I skimmed some of it and it mostly looks reasonable. You may want to wait for #448 which will remove at least some of the changes needed. It's waiting for a new version of the Sail compiler to be released.
@Timmmm It seems like the changes needed to either PR based on each other should be relatively minor, so it probably doesn't matter the order they are merged. |
0fee98f
to
d054ef7
Compare
@Timmmm Thanks for all your earlier comments. I believe I’ve taken care of all of them at this point. I’d appreciate it if you could give this another look. Aside from more rigorous testing (which is being worked on) I think this PR is ready. |
dfd2a20
to
bb322dd
Compare
7a55de8
to
167bbf8
Compare
model/riscv_fdext_regs.sail
Outdated
if (sizeof(flen) == 32) | ||
then 0x_FFFF @ val_16b | ||
else 0x_FFFF_FFFF_FFFF @ val_16b | ||
match (sizeof(flen)) { |
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.
These boxing/unboxing functions can at least be generic, as in my other commit.
However, unless you are in a hurry for this I kind of feel like it would still be worth:
- Waiting for the new float support in the Sail standard library.
- Update my other MR to use that & make the code more generic.
- Then update this.
That may take a while I'm afraid but I guess you'd noticed that's pretty normal for this project! (Also I'm not the boss here so feel free to seek a second opinion!)
I think we should re-evaluate when there's a new Sail compiler release, which I imagine shouldn't be too long.
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 know there is work being done on quad tests for riscv-arch-test which will want to use the sail model to generate signatures. If those are completed before we get all of the floating point updates into the Sail standard library and make the code more generic, then we may want to revisit merging this sooner but otherwise, I agree.
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.
@jordancarlin Do you know if anyone is planning to make and tape-out a real chip that implements the Q extension?
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.
@martinberger The OpenHW Core-V-Wally processor (https://github.com/openhwgroup/cvw) implements the Q extension in some of its configurations. There is an upcoming tape-out planned for it very soon. I'm double checking which configuration is planned for that, but it sounds like it might include Q.
d136b9e
to
9e66fc3
Compare
e8db0dc
to
39fc093
Compare
78756d5
to
4f15ea4
Compare
Complete Q extension implementation including Zfa instructions Based on original work by 'LiuTaowen-Tony' in riscv#187
Adds support for all Q extension instructions plus Q related Zfh and Zfa instructions.
size_enc
is also updated to have a 3 bit version (size_enc_wide
) that supportsQUAD
word widths.Fixes #187 from @LiuTaowen-Tony. There were lots of merge conflicts from the past 2 years. This resolves them and updates the changes that were made to comply with current conventions (removes effects, compatibility with existing Zfh and Zfa extensions that were added since the original version was written). It also changes the core soft-float interface to use two 64 bit values to avoid the need to create MPZ variables as was done in #187.