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

Allow component declaration in VHDL blocks #2255

Merged
merged 4 commits into from
Jun 24, 2022
Merged

Conversation

vmchale
Copy link
Contributor

@vmchale vmchale commented Jun 22, 2022

This allows us to handle component declarations in VHDL blocks.

It it necessary for VHDL support for IP as we use it, see #2182 (comment)

Still TODO:

  • Check copyright notices are up to date in edited files

@vmchale vmchale marked this pull request as ready for review June 22, 2022 17:32
@vmchale vmchale mentioned this pull request Jun 22, 2022
10 tasks
Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

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

LGTM from a purely functional POV. I've got a few engineering suggestions though.

Edit: Also usually I'd ask for a test, but I think we're implicitly going to do that for all cores in clash-cores in the near future :).

@DigitalBrains1
Copy link
Member

I also noticed something I'd like to point out but I will do that tomorrow.

@vmchale vmchale force-pushed the blackbox-vhdl branch 2 times, most recently from 369fcbe to 09b97e9 Compare June 23, 2022 00:18
Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

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

LGTM, curious what @DigitalBrains1 has to say!

@vmchale vmchale requested a review from alex-mckenna June 23, 2022 14:55
@vmchale
Copy link
Contributor Author

vmchale commented Jun 23, 2022

I've rewritten with Alex's suggestions - I think this should be squashed when it is merged.

@martijnbastiaan martijnbastiaan enabled auto-merge (squash) June 24, 2022 14:03
@martijnbastiaan martijnbastiaan merged commit afe5f57 into master Jun 24, 2022
@martijnbastiaan martijnbastiaan deleted the blackbox-vhdl branch June 24, 2022 14:30
@vmchale vmchale restored the blackbox-vhdl branch June 24, 2022 14:55
@vmchale vmchale mentioned this pull request Aug 12, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants