symbiyosys: honour include paths #371
Closed
Conversation
This file contains 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
The current `run` step of the symbiyosys backend generates a `sby` command which includes the hardcoded `-d build` argument. However, this command line argument is incompatible with multiple tasks.
This fix removes the hardcoded `-d` argument and introduces a new `extra_options` tool option for the symbiyosys backend, which can be used to pass additional arguments for the `sby` command (e.g. `-d build`, if necessary).
The example usage of the `extra_options`:
```
symbiyosys:
tasknames:
# A list of task names to pass to sby. Defaults to empty (in which case
# sby will run each task in the .sby file)
- my_proof
extra_options:
# A list of extra command line arguments to sby. Defaults to empty.
- -d
- build
```
|
@Henkru Can i have your thoughts also please? |
ollie-etl
commented
Mar 25, 2023
| @@ -205,7 +246,6 @@ def _get_read_flags(self): | |||
| "-D{}={}".format(key, self._param_value_str(value)) | |||
| for key, value in self.vlogdefine.items() | |||
| ] | |||
| + ["-I{}".format(inc) for inc in self.incdirs] | |||
There was a problem hiding this comment.
No longer do includes with flags. This doesn't work
ollie-etl
commented
Mar 25, 2023
| @@ -275,7 +315,7 @@ def _interpolate_sby(self, src): | |||
| "as a Jinja2 template: {}.".format(src_path, err) | |||
| ) | |||
|
|
|||
| files = "\n".join(self.rtl_paths) | |||
| files = "\n".join(map(lambda x: f"{x.rel_name} {x.name}", self.includes)) + "\n" + "\n".join(self.rtl_paths) | |||
There was a problem hiding this comment.
update the files template to contain include files in dst src form
ollie-etl
commented
Mar 25, 2023
| ], | ||
| } | ||
|
|
||
| def _get_include_files(self, force_slash=False): |
There was a problem hiding this comment.
We need the include file name, no just the include directory
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR is on top of #362, because symbiyosys is basically unusable without.
symiyosys tasks are performed within a sub directory of the working directory, and symiyosys copies all working source into that directory. Currently include files are not specified in the
[files]section of the file, so are not copied in to that working set.This PR changes that, and generates entries for the include files. This are of the lesser used form
By using this form, we can honour the
include_pathproperty in the include files.so includes with paths in files can be honoured
a.sva.coregenerates
The file structure within the symnbiyosys working directory now contains includes at the correct relative locations