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

read_place: do not throw error if block name is invalid #1541

Merged
merged 1 commit into from Sep 15, 2020

Conversation

acomodi
Copy link
Collaborator

@acomodi acomodi commented Sep 14, 2020

Signed-off-by: Alessandro Comodi acomodi@antmicro.com

Description

This fixes an issue when reading placement files.
Placement files can be read for two different reasons:

  1. Prior to the routing stage if the flow is divided in three diffrent calls to VPR P&R stages
  2. To read placement constraints files.

In SymbiFlow, there are a set of tests which are automated and the verilog design alongside with the PCF constraints is automatically generated. The PCF constraints will translate into a placement constraints file.

The issue is that, during the various test design generation, some parameters may allow for some IO signals to be unconnected after synthesis, conflicting with the fact that VPR throws an error if a block specified in the placement constraints file is not present in the packed netlist.

Instead of throwing an error, this PR downgrades the error to a warning.

Another possible solution would be to encapsulate the check in a sub-routine that can be disabled at run-time with a flag (

gen_grp.add_argument<std::string>(args.disable_errors, "--disable_errors")
.help(
"Parses a list of functions for which the errors are going to be treated as warnings.\n"
"Each function in the list is delimited by `:`\n"
"This option should be only used for development purposes.")
.default_value("");
).
This would require a sub-routine as the errors would otherwise be disabled in all the original routine, and those are errors which should not be disabled.

Related Issue

How Has This Been Tested?

SymbiFlow tests

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Signed-off-by: Alessandro Comodi <acomodi@antmicro.com>
@probot-autolabeler probot-autolabeler bot added lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool labels Sep 14, 2020
@vaughnbetz
Copy link
Contributor

Downgrading to a warning is fine. Code looks fine. Can be merged as soon as CI goes green.

@mithro
Copy link
Contributor

mithro commented Sep 14, 2020

@acomodi -- I'm not sure this is the correct solution to the problem here? Why are we getting invalid block names?

@acomodi
Copy link
Collaborator Author

acomodi commented Sep 14, 2020

@mithro Specifically, the error I get is when running the ice40 carry-stress tests.

For what I understand those tests are generated automatically for each board, as well as the PCF files. For some of those tests it happens that, after synthesis, some of the inputs remain unconnected, therefore not added to the packed netlist.

The constraints file though contain those IO top level pins, generating the error described above.

Before recent changes applied to the ways placement files are being read and processed, if a block specified in the placement file was not found in the netlist, it resulted in a warning as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants