-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add dimensional and type data to yxi
outputs
#1964
Conversation
Adds memory_type ((seq or comb), dimension count, dimension sizes, and total size TODO: Fix dpeendent import issues TODO: Test & change runt outputs
Can now build correctly when passing `--features serialize` flag. Normal compilation does not work
wrt feature gating, I think that the simplest option might just be to add an I prefer one of the above over making the axi backend a no-op when the serialization feature is not used since that seems needlessly confusing. Regardless, we don't want to have an ungated dependency on I don't know if @sampsyo has thoughts on this |
I think we should eventually move AXI out of thr compiler and mame it a standalone tool |
Sorry for the late reply on this, the new JSON fields are great!! This has all the info I need for my testbench creation script 🙂 |
I think the dependencies look better now, could either/both of you confirm? @EclecticGriffin @rachitnigam |
yxi
outputsyxi
outputs
ddd571e
to
f5e7b8d
Compare
This reverts commit e71886a.
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.
Most changes seems fine except the one comment I left. Overall, I'd like for YXI to be a separate tool from the Calyx compiler because it doesn't need the rest of the optimization infrastructure.
Dockerfile
Outdated
@@ -54,7 +54,7 @@ WORKDIR /home | |||
RUN git clone https://github.com/cucapra/dahlia.git | |||
WORKDIR /home/dahlia | |||
## Checkout specific version. Fetch before checkout because clone might be cached. | |||
RUN git fetch --all && git checkout 88e05e5 | |||
RUN git fetch --all && git checkout master |
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.
Don't do this. We need to keep the Dahlia version pinned.
Thanks for taking a look. |
* Tried to change yxi utils and output Adds memory_type ((seq or comb), dimension count, dimension sizes, and total size TODO: Fix dpeendent import issues TODO: Test & change runt outputs * cfgs * Add crf[...serialize...] in calyx-ir utils. Can now build correctly when passing `--features serialize` flag. Normal compilation does not work * Update runt tests * Refactoring * run clippy * update cargo.tomls to add an yxi feature * formatting * change toplevel xilinx backend to correctly get total size of memories * change dockerfile to build with * change dahlia checkout to latest commit in master in Dockerfile * add --all-features to cargo build in .github/workflows/rust.yml * Revert "change dockerfile to build with" This reverts commit e71886a. * revert dockerfile to pinned dahlia checkout --------- Co-authored-by: Rachit Nigam <rachit.nigam12@gmail.com>
This PR adds memory-type (
Sequential
orCombinational
) data, explicit dimensionality sizes andidx
sizes, and makesyxi
look for both@external
memories andref
memories (before it was limited to just@external
memories). #1932I would be happy to hear any thoughts on the naming of the information output, or if there is something we wanted to be output that is being missed @ayakayorihiro.
Currently a draft because this doesn't compile correctly with a normal
cargo build
.This is because the
yxi
backend is not gated behind any feature flags, but relies oncalyx-ir/utils
to have access toserde
, which is gated behind--features serialize
. So I guess we can either gateyxi
behind--features serialize
or--features yxi
or remove the gating ofserde
incalyx-ir/utils
. It might be worth noting that theyxi
backend already has a non-gated dependency onserde
. Dunno if that's the way things should be.cc @rachitnigam @EclecticGriffin Any thoughts on best way to proceed with the feature gating issue?