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

Add quads to float lib #574

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

jordancarlin
Copy link
Contributor

This PR extends the fp types that have been created so far to support 128 bits and extends the existing test files to confirm correct operation on quads.

There is ongoing work to add quads to the riscv model (riscv/sail-riscv/pull/445), so including them in the float library that is being created seems like a good idea.

@jordancarlin
Copy link
Contributor Author

Updated to include changes to float_eq for 128 bit floats.

@jordancarlin
Copy link
Contributor Author

@Alasdair It would be great to get this merged so that it doesn't fall behind as more features are added to the float library.

@Incarnation-p-lee I'm happy to follow up with quad related addendums to your PRs, or feel free to include quad work in your PRs from the get go, whatever is easier for you. I just want to make sure that the final version of the sail float library has full quad floating point support so that the quad implementation in the RISC-V Sail model can use it when we switch over.

@Incarnation-p-lee
Copy link
Contributor

Incarnation-p-lee commented Jun 17, 2024

@Alasdair It would be great to get this merged so that it doesn't fall behind as more features are added to the float library.

@Incarnation-p-lee I'm happy to follow up with quad related addendums to your PRs, or feel free to include quad work in your PRs from the get go, whatever is easier for you. I just want to make sure that the final version of the sail float library has full quad floating point support so that the quad implementation in the RISC-V Sail model can use it when we switch over.

Thanks @jordancarlin for making this happen, will include quad from the underlying PR in the test.

@Incarnation-p-lee
Copy link
Contributor

Incarnation-p-lee commented Jun 17, 2024

Hi @Alasdair,

Could you please help to do me a fever that merge this PR before #587 ? I may need to rebase for quad support.

@jordancarlin
Copy link
Contributor Author

Since #587 was merged first, I can go ahead and rebase this one tonight, and then going forward you can include quads if that works for you @Incarnation-p-lee.

@Incarnation-p-lee
Copy link
Contributor

Since #587 was merged first, I can go ahead and rebase this one tonight, and then going forward you can include quads if that works for you @Incarnation-p-lee.

That makes much sense to me. Thanks again and will continue the rest floating API(s) with quad (aka 128 bits) support.

@jordancarlin
Copy link
Contributor Author

@Alasdair this has been rebased and should be ready to merge now. Ideally before more of the float lib is added.

@jordancarlin
Copy link
Contributor Author

@Incarnation-p-lee Thanks for taking on 128 bit support with the rest of the float APIs. Let me know if you need any other help with it.

@Incarnation-p-lee
Copy link
Contributor

@Incarnation-p-lee Thanks for taking on 128 bit support with the rest of the float APIs. Let me know if you need any other help with it.

Thanks a lot. I will plan to improve the test data of float lib after this patch merged.

Copy link

Test Results

    9 files  ±0     20 suites  ±0   0s ⏱️ ±0s
  649 tests ±0    649 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 076 runs  ±0  2 075 ✅ ±0  1 💤 ±0  0 ❌ ±0 

Results for commit 2d6e0d6. ± Comparison against base commit 387771d.

@Alasdair Alasdair merged commit d00c025 into rems-project:sail2 Jun 21, 2024
3 checks passed
@Alasdair
Copy link
Collaborator

Thanks!

@jordancarlin jordancarlin deleted the add_quads_to_float_lib branch June 21, 2024 23:41
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