-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
tools/metrics.py: Add VIRT_RV32 to the code size metrics. #15843
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15843 +/- ##
=======================================
Coverage 98.57% 98.57%
=======================================
Files 164 164
Lines 21341 21341
=======================================
Hits 21036 21036
Misses 305 305 ☔ View full report in Codecov by Sentry. |
Code size report:
|
0ef3c89
to
857d67a
Compare
This won't actually be reported by the github-actions bot (see its comment immediately above) unless "v" is added to the |
Sorry, I thought I added the second commit when I force updated the branch last night. The second commit is now part of the PR. |
The CI for the code size check is failing, but I think that will only work once this is merged, so that it has the "v" option on master to check the PR against. |
@agatti are you sure this will work, won't it need to install a RISC-V toolchain in |
084ef38
to
e200340
Compare
You're right, turned out I tested that with a VM that had already built the RV32 code once. This should now be addressed. Edit: Wait, that runs on 20.04? Is it OK if I update the workflow to use 22.04 or is there a particular reason that workflow is still on that? |
See commit 0a07915 ... something to do with gcc-multilib. You're welcome to try updating that to 22.04, but it would need to be a separate PR because that CI step needs to be tested, and it won't be tested here because it fails due to missing "v" support. |
That should be fixed in #15884 - besides the RV64 and macOS failures (expected and occasional), the code size task seems to work just fine on 22.04. |
This commit adds the Qemu-based RISC-V 32 bits `VIRT_RV32` board to the list of ports/boards to be built for measuring code size changes. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
This commit adds the `VIRT_RV32` board to the list of targets for calculating code size changes as part of the CI pipeline. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
e200340
to
3e085c3
Compare
Summary
This commit adds the Qemu-based RISC-V 32 bits
VIRT_RV32
board to the list of ports/boards to be built for measuring code size changes, as per the discussion in #15743 (comment).Testing
Ran the tool locally on a CI-like environment.
Trade-offs and Alternatives
In the original discussion it was mentioned the potential for the metrics collection step to take too long. The RV32 port in question brings in as little third party code as possible and it should be relatively quick to build, all things considered (a single full build takes around a minute on an underclocked i5 laptop). However, until there are real measurements being taken in the final environment this is all speculation.