-
Notifications
You must be signed in to change notification settings - Fork 294
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
Simplify handling of Liberty file related variables #2619
base: master
Are you sure you want to change the base?
Simplify handling of Liberty file related variables #2619
Conversation
…SC_LIB_FILE Signed-off-by: Emil J. Tywoniak <emil@tywoniak.eu>
e7da6d6
to
d0dee53
Compare
This can be followed up by integrating YosysHQ/yosys#4797 and redoing #2139 if possible |
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.
Thanks for the work on this!
@@ -23,7 +23,7 @@ proc write_keep_hierarchy {} { | |||
if {[env_var_exists_and_non_empty DFF_LIB_FILE]} { | |||
dfflibmap -liberty $::env(DFF_LIB_FILE) | |||
} else { | |||
dfflibmap -liberty $::env(DONT_USE_SC_LIB) | |||
dfflibmap -liberty $::env(SC_LIB_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 only reason for this if/else is because dfflibmap
used to only take one liberty file. If it accepts multiple now, then you can simply pass in LIB_FILES_EXTRACTED
and eliminate the DFF_LIB_FILE
variable entirely. In fact, DFF_LIB_FILE
was added exclusively to support ASAP7, no other PDK uses it.
@@ -67,7 +67,7 @@ foreach cell $::env(DONT_USE_CELLS) { | |||
if {[env_var_exists_and_non_empty DFF_LIB_FILE]} { | |||
dfflibmap -liberty $::env(DFF_LIB_FILE) {*}$dfflibmap_args | |||
} else { | |||
dfflibmap -liberty $::env(DONT_USE_SC_LIB) {*}$dfflibmap_args | |||
dfflibmap -liberty $::env(SC_LIB_FILE) {*}$dfflibmap_args |
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.
Same
$(LIB_FILES_EXTRACTED): $$(filter %$$(@F) %$$(@F).gz,$(LIB_FILES)) | ||
@mkdir -p $(OBJECTS_DIR)/lib | ||
$(UTILS_DIR)/preprocessLib.py -i $^ -o $@ | ||
@if echo "$<" | grep -q "\.gz$$"; then \ | ||
gunzip -c $< > $@; \ | ||
else \ | ||
cp $< $@; \ | ||
fi |
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.
Does Yosys support reading in compressed .gz files and decompressing on-the-fly? This step could be entirely eliminated if so.
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.
It does not, but it could. It requires zlib by default to support .fst wave files. I'll see what I can do
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.
I think it does when you supply the file to a frontend (e.g. read_liberty
) but not e.g. in dfflibmap -liberty
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.
It would be nice to support gz in both
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.
We're working on it
original_pin
used in Liberty file pin declarations of cells it actually uses to map to even iforiginal_pin
is not defined as user-defined. Probably from Synopsys PrimeTime folklore? Can't find much of a trace. Let me know if private PDKs break with thisdont_use
attributes to some of the cells in those filesobjects/
I tried this out with asap7 and nangate and it seems to work fine
cc @povik