Include nested FINNLoop sources for stitched rtlsim#1617
Conversation
(cherry picked from commit 5be9aad)
STFleming
left a comment
STFleming
left a comment
There was a problem hiding this comment.
Thanks for the PR @ollycassidy13!
I've left some comments inline, please reach out if anything doesn't make sense.
| ReplaceVerilogRelPaths, | ||
| ) | ||
| from finn.util.basic import make_build_dir, resolve_xilinx_tool | ||
| from finn.util.basic import getHWCustomOp, make_build_dir, resolve_xilinx_tool |
| def append_missing_finnloop_rtlsim_sources(model, v_file_list): | ||
| """Add nested FINNLoop-generated HDL sources to the top rtlsim source list. | ||
|
|
||
| Vivado's top-level USED_IN_SYNTHESIS query can omit HDL generated under a |
There was a problem hiding this comment.
When you say "can omit" is this a random sporadic error that you were observing? Or is it consistently missing sources in a particular scenario?
The reason I ask is because on finn/dev the test_finnloop_end2end_mlo tests are passing, wouldn't we expect these tests to fail if there were sources missing, perhaps this has already been addressed on dev?
|
|
||
| @pytest.mark.fpgadataflow | ||
| def test_ipstitch_appends_missing_finnloop_rtlsim_sources(tmp_path, monkeypatch): | ||
| top_list = tmp_path / "all_verilog_srcs.txt" |
There was a problem hiding this comment.
This test proves that for a hand-constructed set of source lists the helper appends the expected HDL entries.
However, what I think would be a stronger test is justifying that the helper function is needed. What would be stronger and prove this is an integration test where we build a real FINNLoop stitched IP and assert that after CreateStitchedIP the compiled XSI object contains / can elaborate the nested modules. Ideally this test would show that without the call to append_missing_finnloop_rtlsim_sources it is failing and then when the call is added it is passing.
| FakeNode("FINNLoop", str(tmp_path / "missing_loop")), | ||
| ] | ||
| ) | ||
| monkeypatch.setattr( |
There was a problem hiding this comment.
This monkey patching might disappear after fixing getHWCustomOp
| def append_missing_finnloop_rtlsim_sources(model, v_file_list): | ||
| """Add nested FINNLoop-generated HDL sources to the top rtlsim source list. | ||
|
|
||
| Vivado's top-level USED_IN_SYNTHESIS query can omit HDL generated under a |
There was a problem hiding this comment.
When you say "can omit" is this something that happens unpredictably with the underlying tools, or it will always always happen in specific scenarios?
2 participants
Adds missing nested FINNLoop HDL sources to the stitched-IP rtlsim source list.
Includes the source-list helper and its focused IP-stitch regression test.
Change type: Python