rems-project / sail

Sail architecture definition language
Other
563 stars 92 forks source link

Add quads to float lib #574

Closed jordancarlin closed 1 week ago

jordancarlin commented 3 weeks ago

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 commented 2 weeks ago

Updated to include changes to float_eq for 128 bit floats.

jordancarlin commented 1 week ago

@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 commented 1 week ago

@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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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

jordancarlin commented 1 week ago

@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 commented 1 week ago

@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.

github-actions[bot] commented 1 week ago

Test Results

    9 files  ±0     20 suites  ±0   0s :stopwatch: ±0s   649 tests ±0    649 :white_check_mark: ±0  0 :zzz: ±0  0 :x: ±0  2 076 runs  ±0  2 075 :white_check_mark: ±0  1 :zzz: ±0  0 :x: ±0 

Results for commit 2d6e0d63. ± Comparison against base commit 387771d6.

Alasdair commented 1 week ago

Thanks!