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

vendor.xilinx_7series: read extra .xdc files. #119

Merged
merged 1 commit into from
Jul 2, 2019

Conversation

peteut
Copy link
Contributor

@peteut peteut commented Jul 2, 2019

I'm not quite sure if this is correct in general but I expect .xdc constraint extra files to be applied. This patch filters extra_files for .xdc endings and applies them using read_xdc.

@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #119 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #119   +/-   ##
=======================================
  Coverage   80.69%   80.69%           
=======================================
  Files          32       32           
  Lines        5036     5036           
  Branches     1090     1090           
=======================================
  Hits         4064     4064           
  Misses        837      837           
  Partials      135      135

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94e8f47...f2cc64f. Read the comment docs.

@whitequark
Copy link
Contributor

Yes, this is reasonable. Are these commands order dependent? Should the extra .xdc files be read before or after the built-in ones?

@peteut
Copy link
Contributor Author

peteut commented Jul 2, 2019

Yes, they are indeed. Additional files may refer to clocks defined in the common .xdc file for instance (set_input_delay, set_output_delay, ...). So I think additional .xdc should be read after the common one.

@whitequark whitequark merged commit b67f5cf into m-labs:master Jul 2, 2019
@whitequark
Copy link
Contributor

I anticipate that we will have more such loops adding extra files, so maybe we should make them nicer. Would you consider submitting another PR that abstracts this into a platform function, something like platform.iter_extra_files(".v", ".sv")? If not, no worries, I can do it myself.

@peteut
Copy link
Contributor Author

peteut commented Jul 2, 2019

I anticipate that we will have more such loops adding extra files, so maybe we should make them nicer. Would you consider submitting another PR that abstracts this into a platform function, something like platform.iter_extra_files(".v", ".sv")? If not, no worries, I can do it myself.

Good point, I'll look into that.

Sorry, something went wrong.

@peteut
Copy link
Contributor Author

peteut commented Jul 2, 2019

@whitequark like 4c61207?
If so I'll update all platforms to employ the iter_extra_files method and create a PR.

Sorry, something went wrong.

@whitequark
Copy link
Contributor

@peteut Pretty much! Just a couple of nits.

Sorry, something went wrong.

@peteut
Copy link
Contributor Author

peteut commented Jul 2, 2019

@peteut Pretty much! Just a couple of nits.

@whitequark thanks for reviewing it, I'll fix it tonight.

Sorry, something went wrong.

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.

None yet

2 participants