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

[fix] fix the build command for symbiyosys #362

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Henkru
Copy link
Contributor

@Henkru Henkru commented Dec 29, 2022

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 (see #257).

If the -d build argument is specified, the output folder of sby is forced to build/<core>/<target>-symbiyosys/build. Otherwise, the following path is used: build/<core>/<target>-symbiyosys/test_<taskname>.

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

Closes #257.

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
```
@@ -108,8 +112,10 @@ class Symbiyosys(Edatool):
"lists": {
# A list of tasks to run from the .sby file. Passed on the sby
# command line.
"tasknames": "String"
}
"tasknames": "String",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this tool_options some legacy stuff? Other edatool classes are using TOOL_OPTIONS with a different structure.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This backend is part of the legacy tool API. The classes under the tools and flows directories belong to the new flow API. Unfortunately most tools haven't been ported over to the new API yet.

@olofk
Copy link
Owner

olofk commented Feb 22, 2023

Thanks. This sounds good to me. Hard coded parameters should be avoided as far as possible. I would like to rename extra_options to sby_options for symmetry with other tools.

ollie-etl added a commit to etlsystems/edalize that referenced this pull request Mar 24, 2023
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.

SymbiYosys Backend Doesn't Support Running All Tasks
2 participants