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: read_lib args and placeholders #4343

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

phsauter
Copy link
Contributor

Adds a way to give read_lib additional arguments.

This can for example be used to define -S <slew> -G <gain> so ABC will calculate its internal delay model from the standard cells liberty data. Without this it uses a unit delay model, which may produce worse results.
An example use:

abc -liberty "$tech_cells" -D $period -script "$script" -liberty_args "-S 20 -G 3" -showtmp

I changed it so the placeholders like {D} are also replaced in user provided scripts. This seems reasonable to me as OpenLane, OpenROAD-flow-scripts etc currently use the same placeholders but have to replace it themself.
Additionally, there is a new placeholder called {tmpdir}, it contains the path to the yosys-abc directory currently being used. This allows users to temporarily store information from their user script.

@@ -849,6 +861,7 @@ void abc_module(RTLIL::Design *design, RTLIL::Module *current_module, std::strin
for (size_t pos = abc_script.find("dretime;"); pos != std::string::npos; pos = abc_script.find("dretime;", pos+1))
abc_script = abc_script.substr(0, pos) + "dretime; retime -o {D};" + abc_script.substr(pos+8);

// replace placeholders in abc.script and user.script
Copy link
Member

Choose a reason for hiding this comment

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

Please factor out the substitution operation so that it's not open-coded many times

} else {
// read in user script
std::ifstream ifs(script_file.c_str());
if(ifs.is_open()) {
Copy link
Member

Choose a reason for hiding this comment

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

Please fix whitespace/indentation in this block

@@ -876,6 +908,13 @@ void abc_module(RTLIL::Design *design, RTLIL::Module *current_module, std::strin
fprintf(f, "%s\n", abc_script.c_str());
fclose(f);

buffer = stringf("%s/user.script", tempdir_name.c_str());
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the abc.script/user.script separation? Why not write both to the same file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The abc.script and user.script separation is how it currently works. It basically separates the setup (loading the network and library and later writing it out) from the actual optimization and mapping script.

A different approach (and the one I would like to move in) is to give an option to not use the abc.script part and instead expect it to be part of the custom user.script and then just replace the placeholders in user.script.

This would give full freedom in what people do with ABC, going as far as invoking multiple ABC scripts in parallel and selecting one based one some criteria before returning to Yosys.

@@ -1743,6 +1782,10 @@ struct AbcPass : public Pass {
liberty_files.push_back(args[++argidx]);
continue;
}
if (arg == "-liberty_args" && argidx+1 < args.size()) {
Copy link
Member

Choose a reason for hiding this comment

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

Document this option

@phsauter phsauter mentioned this pull request Sep 10, 2024
4 tasks
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.

2 participants