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

[impl] add riscv cpu support #69

Merged
merged 2 commits into from
Feb 12, 2022
Merged

[impl] add riscv cpu support #69

merged 2 commits into from
Feb 12, 2022

Conversation

Xeonacid
Copy link
Contributor

@Xeonacid Xeonacid commented Feb 12, 2022

Since https://github.com/uraimo/run-on-arch-action doesn't support riscv currently, I'm not adding any test. I will try hard to add support for it.

@codecov
Copy link

codecov bot commented Feb 12, 2022

Codecov Report

Merging #69 (ed9bf87) into master (63ed9a0) will increase coverage by 0.14%.
The diff coverage is n/a.

❗ Current head ed9bf87 differs from pull request most recent head 3f24366. Consider uploading reports for the commit 3f24366 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
+ Coverage   93.96%   94.10%   +0.14%     
==========================================
  Files          53       53              
  Lines       11427    11381      -46     
==========================================
- Hits        10737    10710      -27     
+ Misses        690      671      -19     
Impacted Files Coverage Δ
src/c4/ext/fast_float_all.h 26.81% <0.00%> (-1.31%) ⬇️
src/c4/charconv.hpp 97.55% <0.00%> (-0.04%) ⬇️
test/test_memory_resource.cpp 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63ed9a0...3f24366. Read the comment docs.

@biojppm
Copy link
Owner

biojppm commented Feb 12, 2022

I had trouble finding any action running on RISC-V. Was hoping you knew of one, even outside run-on-arch.

Anyway, if there is no action, we'll have to stick to the old fashioned (risky) way. Have you tried this changeset yourself? That is, does this changeset enable the c4core tests to successfully run in RISC-V, or is anything else required? You can try it like this:

cmake -S c4core -B c4core-build -D C4CORE_DEV=ON
cmake --build c4core-build --target c4core-test-run

@biojppm
Copy link
Owner

biojppm commented Feb 12, 2022

Also you can update the cmake submodule. It's not required, but makes sense since the changesets are related.

@Xeonacid
Copy link
Contributor Author

@biojppm I find that run-on-arch has an open PR uraimo/run-on-arch-action#58 which adds support for RISC-V. But I doubt when it can be merged, sadly.

Also you can update the cmake submodule. It's not required, but makes sense since the changesets are related.

OK. I will do that soon.

@biojppm
Copy link
Owner

biojppm commented Feb 12, 2022

@Xeonacid thanks for the info on that RISC-V PR; was not aware of it.

To merge this PR, I'm just waiting on your confirmation that this changeset runs the c4core tests in RISC-V, as described above. (The coverage test failures are spurious; everything else looks good).

@Xeonacid
Copy link
Contributor Author

@biojppm I just ran it on Arch Linux RISC-V, and all tests passed except the ones using valgrind. (Valgrind does not support RISC-V now. 😢 )

Logs
Could not find executable valgrind
Looked in the following places:
valgrind
valgrind
Release/valgrind
Release/valgrind
Debug/valgrind
Debug/valgrind
MinSizeRel/valgrind
MinSizeRel/valgrind
RelWithDebInfo/valgrind
RelWithDebInfo/valgrind
Deployment/valgrind
Deployment/valgrind
Development/valgrind
Development/valgrind
Unable to find executable: valgrind
138/138 Test #138: c4core-test-std_vector-valgrind ........***Not Run   0.00 sec

83% tests passed, 23 tests failed out of 138

Total Test time (real) =  12.00 sec

The following tests FAILED:
          6 - c4core-test-preprocessor-valgrind (Not Run)
         12 - c4core-test-type_name-valgrind (Not Run)
         18 - c4core-test-types-valgrind (Not Run)
         24 - c4core-test-szconv-valgrind (Not Run)
         30 - c4core-test-error-valgrind (Not Run)
         36 - c4core-test-error_exception-valgrind (Not Run)
         42 - c4core-test-blob-valgrind (Not Run)
         48 - c4core-test-memory_util-valgrind (Not Run)
         54 - c4core-test-memory_resource-valgrind (Not Run)
         60 - c4core-test-allocator-valgrind (Not Run)
         66 - c4core-test-ctor_dtor-valgrind (Not Run)
         72 - c4core-test-chartraits-valgrind (Not Run)
         78 - c4core-test-enum-valgrind (Not Run)
         84 - c4core-test-bitmask-valgrind (Not Run)
         90 - c4core-test-span-valgrind (Not Run)
         96 - c4core-test-substr-valgrind (Not Run)
        102 - c4core-test-charconv-valgrind (Not Run)
        108 - c4core-test-utf-valgrind (Not Run)
        114 - c4core-test-format-valgrind (Not Run)
        120 - c4core-test-dump-valgrind (Not Run)
        126 - c4core-test-base64-valgrind (Not Run)
        132 - c4core-test-std_string-valgrind (Not Run)
        138 - c4core-test-std_vector-valgrind (Not Run)

@biojppm
Copy link
Owner

biojppm commented Feb 12, 2022

Thanks @Xeonacid !

@biojppm biojppm merged commit 79c15b0 into biojppm:master Feb 12, 2022
@biojppm biojppm mentioned this pull request Feb 12, 2022
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.

2 participants