[HW] Adding support for Fixed-Point vector instructions

[hossein1387]

Description of PR that completes issue here...

Changelog

• Support for Fixed-Point vector instructions: vaadd, vaaddu, vsadd, vsaddu, vssub, vssubu, vasub, vasubu
• Two status register for fixed-point unit: vxrm, vxsat

Fixed

• Description of changes

Added

• Support for Fixed-Point vector instructions: vaadd, vaaddu, vsadd, vsaddu, vssub, vssubu, vasub, vasubu
• Two status register for fixed-point unit: vxrm, vxsat

Checklist

• [x] Automated tests pass
• [x] No frequency degradation
• [x] Changelog updated
• [x] Code style guideline is observed

Please check our contributing guidelines before opening a Pull Request.

[mp-17]

Hey Hossein, thanks for this important contribution and sorry for the delay in the answer ;) I see that the riscv-tests are not completely successful, are you still working on the PR?

[mp-17]

Hello Hossein, thank you for the PR! I have still not checked precisely the assignments within the simd_alu in terms of math correctness, but I started a review process and you can address those points in the meantime.

BTW: can you please restart the CI? You can do that from the "Action" tab, or you can just push another commit (you will need to do it anyway to address some of the review points). Thanks a lot!

[hossein1387]

Hello Hossein, thank you for the PR! I have still not checked precisely the assignments within the simd_alu in terms of math correctness, but I started a review process and you can address those points in the meantime.

BTW: can you please restart the CI? You can do that from the "Action" tab, or you can just push another commit (you will need to do it anyway to address some of the review points). Thanks a lot!

Hi Matteo, I am not able to see the restart option in the action tab. I am not sure if it is related to the privilege level or not. I can re submit the PR.

[mp-17]

Ehy Hossein, do you have any news? ;)

[suehtamacv]

Well actually the CI is failing on vsadd with an Ara configuration which has two lanes.. That is something we should investigate

[M-Ijaz-10x]

Hey @hossein1387 , @suehtamacv We have been working on fixing the RTL failures of vasub, vasubu and vssub vfixed-point instructions, fixing CI check failures of vsadd, vsaddu and vssub instructions, and adding support for vsmul, vssrl, vssra, vnclip and vnclipu vfixed-point instructions. What we have done so far is as follows:

• We have added support for vsmul, vssrl, vssra, vnclip and vnclipu vfixed-point instruction and have tested them by using the provided tests
• We have fixed the RTL failures of vasub, vasubu and vssub vfixed-point instructions
• We have fixed CI check failures of vsadd, vsaddu and vssub instructions
• We have fixed vssub.c test which caused the spike failure

The final testing of all the bug-fixes and RTL additions is on its way. We are planning to push the fixes as soon as we get the final results.

Thanks

[hossein1387]

Hey @hossein1387 , @suehtamacv We have been working on fixing the RTL failures of vasub, vasubu and vssub vfixed-point instructions, fixing CI check failures of vsadd, vsaddu and vssub instructions, and adding support for vsmul, vssrl, vssra, vnclip and vnclipu vfixed-point instructions. What we have done so far is as follows:

• We have added support for vsmul, vssrl, vssra, vnclip and vnclipu vfixed-point instruction and have tested them by using the provided tests
• We have fixed the RTL failures of vasub, vasubu and vssub vfixed-point instructions
• We have fixed CI check failures of vsadd, vsaddu and vssub instructions
• We have fixed vssub.c test which caused the spike failure

The final testing of all the bug-fixes and RTL additions is on its way. We are planning to push the fixes as soon as we get the final results.

Thanks

That is great! are your changes based of this PR?

[M-Ijaz-10x]

Hey @hossein1387 , @suehtamacv We have been working on fixing the RTL failures of vasub, vasubu and vssub vfixed-point instructions, fixing CI check failures of vsadd, vsaddu and vssub instructions, and adding support for vsmul, vssrl, vssra, vnclip and vnclipu vfixed-point instructions. What we have done so far is as follows:

• We have added support for vsmul, vssrl, vssra, vnclip and vnclipu vfixed-point instruction and have tested them by using the provided tests
• We have fixed the RTL failures of vasub, vasubu and vssub vfixed-point instructions
• We have fixed CI check failures of vsadd, vsaddu and vssub instructions
• We have fixed vssub.c test which caused the spike failure

The final testing of all the bug-fixes and RTL additions is on its way. We are planning to push the fixes as soon as we get the final results. Thanks

That is great! are your changes based of this PR?

Yes

[M-Ijaz-10x]

Hey @hossein1387

As our changes are based of this PR, should we add our changes to this PR or create a new PR (Because there are a few major additions and it would be cleaner to create a new PR)?

Thanks!

[hossein1387]

Hey @hossein1387

As our changes are based of this PR, should we add our changes to this PR or create a new PR (Because there are a few major additions and it would be cleaner to create a new PR)?

Thanks!

@mp-17 @suehtamacv what do you think? I am actually working on this issue now but I do not have vsmul, vssrl, vssra, vnclip and vnclipu.

[M-Ijaz-10x]

Hey @hossein1387

Its great that you are already working on resolving the CI check failures issue, we can wait for this PR to be merged into pulp-platform/ARA/main and later on we can add support for vsmul, vssrl, vssra, vnclip and vnclipu vfixed-point instructions in separate PR.

Thanks!

[suehtamacv]

Hey @hossein1387

As our changes are based of this PR, should we add our changes to this PR or create a new PR (Because there are a few major additions and it would be cleaner to create a new PR)?

Thanks!

You could add a new PR whose base is this branch. That would be clean, and then we could merge the two PRs one after the other

[mp-17]

Thank you @M-Ijaz-10x @hossein1387 for the joint effort! In parallel, I am solving the backend problems we meet after synthesis. I would freeze the merge of this PR until then

[mp-17]

We can close timing after place & route, I will check again before merging with the missing fixed-point instructions, but everything seems okay so far! ;-)

[mp-17]

@hossein1387 , I have created a PR on your GitHub to fix the problems with the CI https://github.com/PolyMTL-Gr2m/ara/pull/1. If you merge it, this CI should be fixed as well.

[mp-17]

Switching to: https://github.com/pulp-platform/ara/pull/147 Thanks Hossein!