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

Memory-efficient zlib usage across Liberty file consumers #4834

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

Conversation

widlarizer
Copy link
Collaborator

In ASIC flows, Liberty files provided in the PDK can be very large and are often compressed with gzip. Prior to this PR, the only pass that accepted .lib.gz was read_liberty, just like read_verilog accepts .v.gz etc. -liberty filename arguments to passes like clockgate and dfflibmap didn't use the decompression code.

The decompress_gzip function also has been reading the entirety of the uncompressed file into RAM (into a stringstream). This is now fixed by implementing an gzip_istream much like the pre-existing gzip_ostream. It has one byte of guaranteed unget() which is sufficient for our Liberty parsing. Using gzstream was ruled out because it's LGPL.

The PR carries also some shuffling of things around which should have limited impact on plugin devs since the new kernel/io.h header is included in kernel/yosys_common.h.

This will resolve #4830 but only on the Yosys side. The Yosys abc pass only hands off the paths specified with abc -liberty path to abc but abc doesn't support .lib.gz which is surprising since it does have its own vendored zlib and wrapper around it used for aiger

Copy link
Member

@povik povik left a comment

Choose a reason for hiding this comment

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

I've only reviewed gzip.cc/gzip.h, it looks fine except for the nits

passes/techmap/dfflibmap.cc Outdated Show resolved Hide resolved
kernel/gzip.cc Outdated Show resolved Hide resolved
int bytes_read = Zlib::gzread(gzf, buffer, buffer_size);
if (bytes_read <= 0) {
if (Zlib::gzeof(gzf))
return traits_type::eof();
Copy link
Member

Choose a reason for hiding this comment

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

Reading https://en.cppreference.com/w/cpp/io/basic_streambuf/underflow we might need to do something about "On failure, the function ensures that either gptr() == nullptr or gptr() == egptr." because the invariant of when this function is called is weaker, it is "The public functions of std::streambuf call this function only if gptr() == nullptr or gptr() >= egptr()."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice catch, now I setg(eback(), egptr(), egptr()); before returning eof

kernel/gzip.cc Outdated Show resolved Hide resolved
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.

Support gziped Liberty files
2 participants