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

Add script for Mentor Precision #55

Merged

Conversation

WRoenninger
Copy link

This adds the feature to generate scripts for adding files to Mentor's Precision RTL synthesis tool.

Also this is the first time that I wrote something rust. So feel free to correct this if I made some obvious mistakes / broke the cargo dependencies.

@micprog
Copy link
Member

micprog commented Oct 4, 2021

Hi @WRoenninger, thanks for the PR! Overall looks good, a few minor things:

  • could you run cargo fmt to fix formatting?
  • Looking at the CI, it seems the Cargo.lock reshuffling exposes an issue with the updated bitflags dependency (used in clap) in older rust versions <1.46.0. It would probably make sense to update the minimum supported rust version in .github/workflows/ci.yml to 1.46.0.
  • I am not very familiar with Mentor Precision RTL, however I believe it is used for fpga synthesis. Would it make sense to add the fpga target? Alternatively the user would be responsible for adding this target if needed.
  • As I am not very familiar with Mentor Precision, I have not checked if the generated scripts work as expected, however generating scripts worked for me, so if you checked the syntax it should be ok.

@WRoenninger WRoenninger force-pushed the feature-script-mentor-precision branch from f96d52b to 0af9a09 Compare October 4, 2021 17:36
@WRoenninger
Copy link
Author

WRoenninger commented Oct 4, 2021

Hi @micprog, added the suggestions to the PR.

  • Executed cargo fmt
  • Updated rust min version to 1.46.0
  • Added fpga as default target.

As for the syntax of the generated script, was using a patched bender with these changes locally already in my workflow.

On that note, more perhaps of a feature request to be handled separately. I am working with some VHDL sources currently and some codebases use different compilation library names instead of the default work. I think there is not really a way of specifying a compilation library per file group. How big would the effort probably be to add such a feature?

@micprog
Copy link
Member

micprog commented Oct 5, 2021

Thanks @WRoenninger ! LGTM now.

Regarding the use of different libraries, you may want to check out a WIP branch dep_info, where you can generate scripts for individual packages with or without its dependencies, as well as excluding individual packages. If I'm not mistaken you can then map the corresponding dependencies to different libraries, e.g. in a Makefile calling bender. If different behavior is desired, I suggest opening an issue to discuss this further.

@WRoenninger WRoenninger force-pushed the feature-script-mentor-precision branch from 12dec34 to 27aad11 Compare December 7, 2021 18:05
Wolfgang Roenninger added 4 commits December 9, 2021 16:28
Signed-off-by: Wolfgang Roenninger <wroennin@iis.ee.ethz.ch>
Signed-off-by: Wolfgang Roenninger <wroennin@iis.ee.ethz.ch>
Signed-off-by: Wolfgang Roenninger <wroennin@iis.ee.ethz.ch>
Signed-off-by: Wolfgang Roenninger <wroennin@iis.ee.ethz.ch>
@micprog micprog force-pushed the feature-script-mentor-precision branch from 27aad11 to 070ef81 Compare December 9, 2021 15:35
@micprog micprog self-requested a review December 9, 2021 15:36
Copy link
Member

@micprog micprog left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, merging

@micprog micprog merged commit f6bcc53 into pulp-platform:master Dec 9, 2021
paulsc96 pushed a commit that referenced this pull request Sep 10, 2022
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.

2 participants