Pass source directories to Verilator as include paths - #54
Merged
Conversation
Verilator does not search an including file's own directory, so a source that includes a header sitting beside it fails to compile with 'Cannot find include file' unless the directory is given as -I. dau-core adding one shared header broke five benches in dau this way, none of which had any reason to know a header existed -- they listed the same tile sources they always had. Treat every directory contributing a source as an include path. That is what a caller listing those sources already means, and it keeps adopting a header from being a change every downstream bench has to make. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Matches every other cocotb test here: CI runners have no verilator, and a test that needs one must skip rather than fail. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
==========================================
+ Coverage 84.54% 84.69% +0.14%
==========================================
Files 60 61 +1
Lines 9702 9723 +21
Branches 1240 1242 +2
==========================================
+ Hits 8203 8235 +32
+ Misses 1198 1184 -14
- Partials 301 304 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The end-to-end test needs verilator, which CI does not have, so it skips exactly where a regression would land unnoticed. Stand in for the runner and assert what the build was asked for, so the behaviour is checked everywhere the suite runs. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
A test that needs verilator reports its own body as uncovered on every runner that lacks one. tests/integration/ is already omitted from coverage and had no contents; this is what it is for. The unit-level companion keeps the contract checked everywhere the suite runs. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
A bare directory path matches no file; coverage omit takes globs. The entry has been inert since it was written, which went unnoticed while the directory had no contents. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Verilator does not search an including file's own directory, so a source with ```include "types.svh"`` fails with "Cannot find include file" unless the directory is given as
-I. One shared header added in dau-core broke five benches in `dau` this way, none of which had any reason to know a header existed — they listed the same tile sources they always had.Treat every directory contributing a source as an include path. That is what a caller listing those sources already means, and it keeps adopting a header from being a change every downstream bench has to make.
The regression test builds a module whose width comes from a header beside it, and was verified to fail without the fix.
505 pass.