Skip to content
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

[BackendEnd] Add support for verilog units in the experimental backend. #107

Merged
merged 80 commits into from
Aug 13, 2024

Conversation

qianxu1998
Copy link
Collaborator

This pull request adds the following things:

  • Add Verilog implementations for all units in experimental/data/verilog. All module_name, ports_name, and file_name are kept the same.
  • Create the rtl-config-verilog.json file that contains all the configuration info for Verilog implementations.
  • Change write_hdl.sh script to select config file based on the hdl specified by the user.
  • Add supports in export-rtl to satisfy Verilog syntax.
  • Changed rtl-cmpi-generator.cpp to support Verilog syntax when generating cmpi unit for Verilog.

The results for regression_test are the same as the VHDL backend. Details can be found here: Link.

qianxu1998 and others added 30 commits July 22, 2024 17:00
… during Verilog writing. Commented out assertion in verilog modules
Copy link
Member

@lucas-rami lucas-rami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few super minor comments.

Again, install and run clang-format on your code. Some of the C++ files are clearly not formatted correctly.

lib/Conversion/CfToHandshake/CfToHandshake.cpp Outdated Show resolved Hide resolved
include/dynamatic/Support/RTL.h Outdated Show resolved Hide resolved
lib/Support/RTL.cpp Outdated Show resolved Hide resolved
@lucas-rami
Copy link
Member

@Carmine50, @qianxu1998

I was fixing an issue today (that you had reported to me) in the RTL exporter and took the opportunity to integrate a clean solution to the Verilog port-mapping issues discussed above in this PR (with a lot of refactoring around it to make the exporter cleaner overall, even in VHDL).

What this means is that you should be able to untrack your changes to the export-rtl.cpp and RTL.h/cpp from this branch, check that everything works fine (it should, lmk if it does not), and then we can merge this.

Apologies for seemingly creating merge conflicts every 2 minutes; I'm trying to make all of these core features super robust before I stop supporting the project professionally.

@lucas-rami
Copy link
Member

Over the weekend I have also removed the old backend and moved the "experimental" one to the non-experimental part of the repository, creating more conflicts again, sorry 😢

Resolving them and making your files non-experimentl should be a matter of

  1. replicating your fixes to the few VHDL components in the new locations (or simply untrack them and open another PR for those fixes),
  2. moving your Verilog folder and new RTL config to data/ (from data/experimental), taking care of adjusting paths in the config (deleting every occurence of experimental/ should do the trick), and
  3. lightly adapting your modifications to the write-hdl script.

@qianxu1998
Copy link
Collaborator Author

I just resolved the conflicts, but I still have some problems with the Verilog generation flow.

Besides that, I noticed a problem. When testing the fir.dyn script, I got the following errors:

dynamatic> set-dynamatic-path  .
dynamatic> set-src             integration-test/fir/fir.c
dynamatic> compile             
In file included from /home/jianliu/Projects/Dynamatic_adds_on/Verilog/dynamatic/integration-test/fir/fir.c:10:
/usr/include/stdlib.h:32:10: fatal error: 'stddef.h' file not found
   32 | #include <stddef.h>
      |          ^~~~~~~~~~
[FATAL] Failed to compile source to affine
dynamatic> write-hdl 
/home/jianliu/Projects/Dynamatic_adds_on/Verilog/dynamatic/./bin/export-rtl: could not open input file '/home/jianliu/Projects/Dynamatic_adds_on/Verilog/dynamatic/integration-test/fir/out/comp/hw.mlir': No such file or directory
[FATAL] Failed to export RTL (vhdl)
dynamatic> simulate

@lucas-rami
Copy link
Member

lucas-rami commented Aug 13, 2024

I just resolved the conflicts, but I still have some problems with the Verilog generation flow.

Anything I should look at? Do you want to send me steps to reproduce?

Besides that, I noticed a problem. When testing the fir.dyn script, I got the following errors:

This seems like an issue with the Polygeist path. Did you check that the compile script's Polygeist include folder path is pointing to a correct location?

@qianxu1998
Copy link
Collaborator Author

I just resolved the conflicts, but I still have some problems with the Verilog generation flow.

Anything I should look at? Do you want to send me steps to reproduce?

Besides that, I noticed a problem. When testing the fir.dyn script, I got the following errors:

This seems like an issue with the Polygeist path. Did you check that the compile script's Polygeist include folder path is pointing to a correct location?

Thank you! =D I have solved the Verilog generation problem, will let you know if I find anything weird.

@qianxu1998
Copy link
Collaborator Author

Now it looks good on my side, please let me know if anything else needs to be changed =D

@lucas-rami
Copy link
Member

All good on my side! Thanks so much for the huge contribution and apologies for the constant changes of everything 😅

@lucas-rami lucas-rami merged commit 4630e42 into EPFL-LAP:main Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants