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

ABC full custom flow #4592

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

ABC full custom flow #4592

wants to merge 7 commits into from

Conversation

phsauter
Copy link
Contributor

@phsauter phsauter commented Sep 10, 2024

Why?
This is a spiritual successor to #4343.
We are trying to integrate a ABC+Mockturtle flow so we can use both logic optimizers in tandem.
Additionally, I think it makes sense to also try to cover the custom liberty args in the ABC script as in #4343.
This lead me to a more general solution that allows an essentially full customization of an ABC flow. Meaning everything besides the writeout of a snippet and the readback can be customized with whatever scripts the user want.

In principle this would also support the integration of more complex script that can directly invoke multiple ABC scripts and then use something like OpenSTA to evaluate them and hand Yosys the best version back.

How?
To achieve this multiple changes are needed:

  • allow multiple scripts to be passed, one being the main -script and any number of additional -helper, all are copied to the temporary directory
  • introduce additional placeholders (similar to the delay that replaces {D}) defining the directory the script is in and the input and output file location
  • in addition to letting the user replace the ABC exe, the flags are user defined
  • let the user select the file format used for writte out (currently only BLIF) and which should be read back (currently BLIF or Verilog)

Example
An example flow using the above features can be found here:
https://github.com/pulp-platform/croc/blob/53882e795aeffaa1e2b205dfb1a563f466cad6e9/yosys/scripts/yosys_synthesis_mockturtle.tcl#L146
It first calls a shell script here:
https://github.com/pulp-platform/croc/blob/phsauter/experimental/yosys/scripts/abc_mockturtle.tcl

Which in turn calls the ABC and mockturtle scripts.

Points of discussion:

  • Do you think this approach/solution makes sense?
  • Should this be a separate command? ABC could be a specialization of it

Todo:

  • integrate a test
  • refactor structural verilog reader (may be useful for others)
  • decide what should be options and what shoukd be only on scratchpad
  • add more write/read formats (eg AIGER)

@phsauter phsauter marked this pull request as draft September 10, 2024 17:33
@widlarizer
Copy link
Collaborator

Previously, the lack of filesystem support on WASI was discussed in #4571

@whitequark
Copy link
Member

Previously, the lack of filesystem support on WASI was discussed in #4571

Note that WASI does support the filesystem (otherwise Yosys and nextpnr could not work at all), but doesn't support std::filesystem specifically for reasons related to threading.

@phsauter
Copy link
Contributor Author

@whitequark should I manually implement it and just find the last / on UNIX-like and \ on Windows or is there a better way?

@whitequark
Copy link
Member

I manually implement it and just find the last / on UNIX-like and \ on Windows or is there a better way?

Last / on *nix, and last either \ or / on Windows. (This will not do the right thing for UNC paths, but we can probably live with that.)

Windows does recognize / as a valid path separator in paths not starting with \\?\.

@phsauter
Copy link
Contributor Author

Searching for _WIN32 I was able to find implementations to get the filename in the following files:

backends/cxxrtl/cxxrtl_backend.cc
frontends/ast/simplify.cc
frontends/verilog/preproc.cc

It might make sense to move the implementation from backends/cxxrtl/cxxrtl_backend.cc to something like yosys_common.

@whitequark
Copy link
Member

Sounds good to me.

@widlarizer
Copy link
Collaborator

-> #4596

It is still possible to have it in CWD if TMPDIR is set to CWD.
The way it was is not ideal for logging purposes of large projects,
as this can create huge amounts of data best stored on some
scratch disk instead of the main drive.
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.

3 participants