-
Notifications
You must be signed in to change notification settings - Fork 896
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
base: main
Are you sure you want to change the base?
Conversation
@@ -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 |
There was a problem hiding this comment.
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()) { |
There was a problem hiding this comment.
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()); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document this option
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:
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.