-
Notifications
You must be signed in to change notification settings - Fork 653
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
New Hammer #1324
New Hammer #1324
Conversation
For future reference: To replicate the CIRCT issues you can try building a |
For the packed array thing, we can go over this issue for now by setting |
Unfortunately, even with using It would be nice to have Hammer's Yosys plugin call this: https://github.com/zachjs/sv2v, but the pre-built images require quite new glibc... |
I added |
cfc7462
to
9b7b3af
Compare
Hi, these static casts are added when Line 195 in a6921d1
|
…script for private tech plugins
6f70eab
to
efb4b7d
Compare
… Remove redundanct conda reqs
|
||
$(VLSI_RTL): $(RTL_DEPS) | ||
ifneq ($(CUSTOM_VLOG), ) | ||
> $(VLSI_RTL) |
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.
Seems like something was deleted here
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.
No. This writes a blank file. Equivalent to echo -n > $(VLSI_RTL)
.
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.
Hm, turns out this breaks in csh-based shells (it should be : > $(VLSI_RTL)
) but our scripts require bash anyway. Good to know for the future.
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.
LGTM. this works for me as of b937a27 (unfortunately don't have time to bump further right now).
I also pushed one small tweak to the Makefile -- I couldn't suggest change because that part of the Makefile was otherwise untouched. Feel free to revert if there's some problem with the change.
This PR switches the VLSI flow over to new Hammer, which is now published on PyPI: https://pypi.org/project/hammer-vlsi/.
This main package is installed via conda, and plugins packages are installed with
pip -e
using theinit-vlsi.sh
script.Several bugs/problems are caused by the migration to CIRCT, and should be investigated:
SystemVerilog packed arrays are being emitted, despite adding thedisallowPackedArrays
lowering flag. This means Yosys is unable to synthesize designs, and there is no roadmap/effort there to support multidimensional arrays. Thus, the Sky130 + OpenROAD tutorial therefore does not work anymore.Fixed by [HWLegalizeModules] Legalize aggregate constant llvm/circt#4626
Async resets are being emitted in a way that synthesis tools like Genus do not like. Specifically, we get something of the form:This is not allowed because the reset in theif(...
should be the one in the sensitivity list.Attempts to use theexprInEventControl
lowering flag have not solved this.The workaround is to manually changeif(~_some_reset)
toif(reset)
. If we are sticking with this workaround, I will update the documentation.Fixed by [ExportVerilog][PrettifyVerilog] Fix exprInEventControl llvm/circt#4625
Line 64 ofvlsi/Makefile
is an ugly hack to separate out the harness blackboxes that cannot be input to synthesis tools due to the presence of DPI calls. It would be best to investigate ways to restore the top/harness blackbox separation in CIRCT.Fixed by @joey0320
Finally, these dependencies need to be included:
Related PRs / Issues:
Type of change:
Impact:
Contributor Checklist:
main
as the base branch?changelog:<topic>
label?changelog:
label?.conda-lock.yml
file if you updated the conda requirements file?Please Backport
?