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

Comprehensive F extension support with SoftFloat integration #260

Merged
merged 6 commits into from
Nov 11, 2023

Conversation

visitorckw
Copy link
Collaborator

@visitorckw visitorckw commented Nov 10, 2023

This PR aimed at achieving full F extension support within rv32emu. It incorporates the SoftFloat open-source library into the rv32emu project to provide comprehensive support for the F extension.

The key changes in this PR include:

  1. Integrated the SoftFloat library into rv32emu, completing the F extension support within the project.
  2. Introduced F extension testing in the CI pipeline, ensuring thorough validation of the implemented functionality.
  3. Updated the README to correct the description of F extension support from incomplete to full support.

Close: #78

@jserv
Copy link
Contributor

jserv commented Nov 11, 2023

Is it possible to use git submodule instead of copying SoftFloat files in this repository?

@jserv
Copy link
Contributor

jserv commented Nov 11, 2023

The key changes in this PR include:

  1. Integrated the SoftFloat library into rv32emu, completing the F extension support within the project.

I would like to revoke my opinion on the implementation of the RISC-V F extension. Integrating SoftFloat would achieve full compatibility and reduce maintenance efforts. There is no need to retain the original RV32F-specific code. That means the conditional build between the original and SoftFloat-specific code can be removed, and let's focus on SoftFloat.

@visitorckw
Copy link
Collaborator Author

Is it possible to use git submodule instead of copying SoftFloat files in this repository?

Currently, I've copied all files as I added additional files in the SoftFloat folder to comply with the risc-v F extension standard. Perhaps we can explore alternatives, such as copying these files into the SoftFloat directory during compilation.

@visitorckw
Copy link
Collaborator Author

I would like to revoke my opinion on the implementation of the RISC-V F extension. Integrating SoftFloat would achieve full compatibility and reduce maintenance efforts. There is no need to retain the original RV32F-specific code. That means the conditional build between the original and SoftFloat-specific code can be removed, and let's focus on SoftFloat.

Sounds like a good idea, I'll make the change later.

@jserv
Copy link
Contributor

jserv commented Nov 11, 2023

Currently, I've copied all files as I added additional files in the SoftFloat folder to comply with the risc-v F extension standard. Perhaps we can explore alternatives, such as copying these files into the SoftFloat directory during compilation.

To incorporate external source code, please refer to the Makefile and examine the sections related to mini-gdbstub. These sections contain information on how to utilize git submodule and their associated build rules.

Makefile Outdated Show resolved Hide resolved
.gitmodules Outdated Show resolved Hide resolved
src/riscv.h Outdated Show resolved Hide resolved
@jserv
Copy link
Contributor

jserv commented Nov 11, 2023

Can you review src/softfloat.h and determine whether we should retain it (partially) or not?

@visitorckw visitorckw force-pushed the full-F-extension-support branch 2 times, most recently from 84f93ac to d31e18c Compare November 11, 2023 15:53
@visitorckw
Copy link
Collaborator Author

Can you review src/softfloat.h and determine whether we should retain it (partially) or not?

We can remove the is_snan function as the softfloat library already provides the same functionality with isSignalingNaN. However, the rest of the content needs to be retained. Additionally, I'm considering whether renaming src/softfloat.h could prevent confusion.

@visitorckw
Copy link
Collaborator Author

I'm uncertain why the current code is failing the Codacy Security Scan test.

.gitmodules Outdated Show resolved Hide resolved
src/softfloat.h Outdated Show resolved Hide resolved
@jserv
Copy link
Contributor

jserv commented Nov 11, 2023

I'm uncertain why the current code is failing the Codacy Security Scan test.

It seems that something is wrong with Docker Hub.

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@sysprog21 sysprog21 deleted a comment from github-advanced-security bot Nov 11, 2023
@sysprog21 sysprog21 deleted a comment from github-advanced-security bot Nov 11, 2023
@sysprog21 sysprog21 deleted a comment from github-advanced-security bot Nov 11, 2023
@jserv
Copy link
Contributor

jserv commented Nov 11, 2023

When rv32emu is built without F extension, i.e., make ENABLE_EXT_F=0, I got the following error messages:

src/riscv.h:9:10: fatal error: softfloat/softfloat.h: No such file or directory
    9 | #include "softfloat/softfloat.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix build without F extension support.

This commit integrate the SoftFloat library into rv32emu, enabling
comprehensive support for the F extension. The inclusion of SoftFloat
ensures the complete implementation of F extension functionality within
rv32emu, meeting the necessary standards and providing enhanced
capability for floating-point operations.
@visitorckw
Copy link
Collaborator Author

Apologies for the error. I've corrected it. It should be resolved now.

src/riscv.h Dismissed Show dismissed Hide dismissed
src/riscv.h Dismissed Show dismissed Hide dismissed
@jserv jserv merged commit 531ad51 into sysprog21:master Nov 11, 2023
20 checks passed
@jserv
Copy link
Contributor

jserv commented Nov 11, 2023

Thank @visitorckw for contributing!

@visitorckw visitorckw deleted the full-F-extension-support branch November 11, 2023 20:43
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.

any idea why instruction like fadd is not passing some of the test cases such as fadd_b10-01 ?
2 participants