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 RISC-V V extension support and add R-V V isal_adler32 #314

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hleft
Copy link

@hleft hleft commented Feb 27, 2025

Include V extension checks at both build and runtime, and optimize isal_adler32

banana_f3:
        new: adler32_warm: runtime =    3062612 usecs, bandwidth 3861 MB in 3.0626 sec = 1261.01 MB/s
        old: adler32_warm: runtime =    3062505 usecs, bandwidth 1027 MB in 3.0625 sec = 335.64 MB/s

@hleft hleft force-pushed the master branch 2 times, most recently from 8c65861 to 47e7350 Compare February 27, 2025 11:01
Use the base implementations for every function.

Signed-off-by: Daniel Gregory <[email protected]>
Copy link
Contributor

@pablodelara pablodelara left a comment

Choose a reason for hiding this comment

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

Just a small comment. Also, what are your plans on #299?
Thanks

sunyuechi added 3 commits February 28, 2025 20:21
banana_f3:
	new: adler32_warm: runtime =    3062612 usecs, bandwidth 3861 MB in 3.0626 sec = 1261.01 MB/s
	old: adler32_warm: runtime =    3062505 usecs, bandwidth 1027 MB in 3.0625 sec = 335.64 MB/s

Signed-off-by: sunyuechi <[email protected]>
When using RISC-V GCC 14, `gcc -O0` passes the test, but `gcc -O2` fails.

The log shows that it enters the branch `if (c_dut != c_ref) {`

even though `c_dut` and `c_ref` have the same value.

Adding `volatile` allows the test to pass.

Signed-off-by: sunyuechi <[email protected]>
@hleft
Copy link
Author

hleft commented Feb 28, 2025

It looks like 299 needs to add build-time and runtime detection for the B extension.

Maybe it's better to merge this PR first so that 299 can more easily call the mbin_interface , DEFINE_INTERFACE_DISPATCHER ... implementations here.

@pablodelara
Copy link
Contributor

Thanks @hleft. Our internal CI is throwing this warning:

configure.ac:80: warning: AC_COMPILE_IFELSE was called before AC_USE_SYSTEM_EXTENSIONS

Could you take a look at it and fix it? This is caused by the last commit, I believe.

Signed-off-by: sunyuechi <[email protected]>
@hleft
Copy link
Author

hleft commented Mar 3, 2025

Thanks @hleft. Our internal CI is throwing this warning:

configure.ac:80: warning: AC_COMPILE_IFELSE was called before AC_USE_SYSTEM_EXTENSIONS

Could you take a look at it and fix it? This is caused by the last commit, I believe.

Okay, I have updated its order.

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.

3 participants