Initial code and tests for Calyx-to-FIRRTL backend #1806
Conversation
calyx-backend/src/firrtl.rs
Outdated
| comps.map_err(|err| { | ||
| let std::io::Error { .. } = err; | ||
| Error::write_error(format!( | ||
| "File not found: {}", | ||
| file.as_path_string() | ||
| )) | ||
| }) |
There was a problem hiding this comment.
So the above and this block could be simplified into the following:
for comp in ctx.components.iter() {
emit_component(comp, out)?
}Alternatively, if you want to preserve the above structure, you could do
let out = emit_component(comp, out)?;But since the return value on success is the empty tuple () aka "unit", there
isn't a need to explicitly return it beyond making sure that we are handling the
errors which can be done via the ? syntax and the Try trait. Since the
CalyxResult can be built from the std::io::Error type (via a From
implementation) the ? automatically handles the conversion between the two
types, freeing you from writing this error translation manually.
There was a problem hiding this comment.
I think this makes sense! But I think I may need to quickly talk this over the next time we meet 😅
calyx-backend/src/firrtl.rs
Outdated
| } | ||
| } | ||
| _ => { | ||
| unreachable!() |
There was a problem hiding this comment.
This should probably be a todo as it is possible to have assignments whose
ports have a group as a parent even though we aren't dealing with them yet. For
context, these ports are the go/done ports attached to groups. So
my_group[done] is a port named done whose parent is a group, rather than a cell.
There was a problem hiding this comment.
I was under the impression that I wouldn't need to cover any ports that have a group as a parent because this backend pass would take place after the compiler removes all groups... What do you think?
There was a problem hiding this comment.
oh yeah, you're totally right my bad. clearly got some wires crossed. You can add a message string to the macro call if you want but otherwise fine to leave as is. Sorry for the confusion!!
There was a problem hiding this comment.
Sounds good! Thanks :)
calyx-backend/src/firrtl.rs
Outdated
| } | ||
| } | ||
| _ => { | ||
| unreachable!() |
There was a problem hiding this comment.
same note as before
calyx-backend/src/firrtl.rs
Outdated
| } | ||
| let mut src_string = String::from(""); | ||
| let source_port = asgn.src.borrow(); | ||
| match &source_port.parent { |
There was a problem hiding this comment.
Since these match blocks are duplicates, it might be worthwhile to abstract
these away into a closure. Something like
let process_port = |port, dest_str| {
...
};
process_port(dest_port, dest_string);
process_port(src_port, src_string);There was a problem hiding this comment.
actually looking at this, it could even be an internal function rather than a
closure since I don't think it needs external stuff, though I might be missing something
There was a problem hiding this comment.
Great point, I actually abstracted both of these into an internal function after making this PR (current changes are in the firrtl-scratch branch) :) Let me bring that internal function over here before I merge.
* Initial commit: added barebones firrtl.rs as a backend option. * Fix formatting errors and try to leverage VerilogBackend * Fix formatting * first pass on inputs and outputs * fix CI formatting issue * more CI formatting fixes * temporary commit before fixing up starter code * Clean up starter firrtl translation code * Fix CI errors * Barebones test containing input/output ports and single assignment * Fix test failure caused by whitespace * clean up unnecessary comments * Address PR comments * Add error message for ports that have groups as parents
I'm working on a backend that translates Calyx programs into FIRRTL, as described in #1552 . This is an initial PR containing some starter code! Specifically, the current backend translates (1) input and output ports, and (2) assignments without guards. I've also added a barebones test program to test this.
The backend can be run via adding the
-b firrtlflag.e.g)
My next steps are outlined in #1805 ! Any suggestions and comments would be very much appreciated 😃