-
Notifications
You must be signed in to change notification settings - Fork 188
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 first party C tests using Zig for cross-compilation #759
Conversation
Add first party C tests that can be used for tests specific to this model. This uses Zig as the compiler since it's small and easy to set up. We could automatically download it for users like we can for GMP (I haven't implemented that yet). This includes a copy of `nanoprinf` for printing to the UART. It has two tests, `test_hello_world.c`, which passes, and `test_max_pmp.c` which fails due to a model bug. There are some TODOs and a lot of it needs cleaning up a fair but bit I wanted some feedback before I continue.
I can't see anything here that justifies using Zig over Clang? You're not even benefiting from its pre-built sysroots. This could easily just be a bunch of Clang invocations generated by CMake itself. |
I have to agree here. I don't see a reason to go with Zig over a standard C compiler. Especially because I assume that most people developing the model will already have a riscv compiler of some sort installed (likely either gcc or clang). While this might not be true for all users (and they won't need it), it seems like a fairy safe assumption for developers. You also mention potentially running these tests by default only in CI, which makes it seem even less necessary to use something like Zig because we can guarantee a compatible version of Clang/GCC in the CI runner. I think it is more likely that developers will run these tests locally before pushing if they don't need to install a (likely new) compiler first. |
It's true we're not taking advantage of the sysroots, but I still think there are some significant advantages:
But... if people are definitely against this then I will change it to use GCC/Clang. |
I opened another PR using Clang instead: #764 As I expected it was a bit painful, but I did get there in the end.
I guess there's some I just commented out the |
Closing now that the pure C version is merged. |
Add first party C tests that can be used for tests specific to this model. This uses Zig as the compiler since it's small and easy to set up. We could automatically download it for users like we can for GMP (I haven't implemented that yet).
This includes a copy of
nanoprinf
for printing to the UART. It has two tests,test_hello_world.c
, which passes, andtest_max_pmp.c
which fails due to a model bug.There are some TODOs and a lot of it needs cleaning up a fair but bit I wanted some feedback before I continue.
Fixes #740