vpr: interchange: add interchange netlist reading support

[acomodi]

Signed-off-by: Alessandro Comodi [email protected]

Description

This PR adds the FPGA Interchange netlist reading and parsing support.

Motivation and Context

With the ongoing development to add support for the netlist interchange, the additional reader for the interchange netlist format is an essential part, thanks to which we can import the circuit according to the format specification.

This is the next relevant addition to the current early support.

How Has This Been Tested?

There is a unit test that contains a simple circuit targetting the interchange test architecture, implemented using the catch2 framework.

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

[tangxifan]

@acomodi Some clean-up and additional comments may be required. See they make sense to you.

[tangxifan]

Is this test deployed to CI? If so, can you remind the workflow/job name?

[acomodi]

Yes, this is under the C++ Unit Test workflow: https://github.com/verilog-to-routing/vtr-verilog-to-routing/runs/4354489589?check_suite_focus=true

[tangxifan]

I just checked the log file of the workflow. I just see 3 tests. Does this test go under any existing tests? Or it should go independently?

[acomodi]

There might be an option to increase verbosity which I may add, but the tests are run under vpr_tests

[tangxifan]

No worries. Thanks for clarification. If we have the verbosity option, it will be helpful.

[acomodi]

So, I have checked a bit the options and unfortunately there is no straightforward way of having a bit more verbose output (e.g. with the single test cases names), we may enable full verbosity (which is way too much) or keep the following, which I think is the best option.

The current setting will output the full log in case of failure though.

[tangxifan]

When reading the vpr/src/base/read_interchange_netlist.cpp, I notice that the net names are used to identify GND and VCC signals in interchangeable files.
So the question is should we put it in the architecture data structure?

• If this is universal for all the FPGA architectures and netlists, it is reasonable.
• If not, should we consider put the constant strings gnd_net and vcc_net in the vpr/src/base/read_interchange_netlist.cpp?

[acomodi]

So, in general, the name of the GND and VCC nets do come from the interchange device description, but they might be absent from the schema, as they are optional. For instance, the Xilinx device have the constant nets specified as GLOBAL_LOGIC_0 and GLOBAL_LOGIC_1.

Given that these net names are variable from arch to arch, they need to be parased from the device file, and this would need to happen in the libarchfpga library.

[tangxifan]

Make sense. Would you mind giving more insights about what client functions require these data?
Currently I see it is used by netlist readers. I am sure it will be used by netlist writers.
When adding the information to the architecture database, we expect they may be used by packer, placer, router as well.

[acomodi]

Indeed, for now I only expect the constant nets to be used in the front-end and back-end only, but it is not excluded that they might come in handy in core algorithms, especially in the packer.

The main reason it is added to the architecture database is because of accessibility of the information stored in the interchange device database and needs to be read in the netlist reader.

[tangxifan]

I see. Maybe it is better to add additional comments to the source file, so that developers know what these variables are.
It is important to clarify

• if the vcc_cell and gnd_cell physically exist in an FPGA fabric/architecture or that they are just virtual. I understand that in commerical FPGAs, there are dedicated circuitry to generate these constant signals.
• if the vcc_net and gnd_net are the required net names for input netlists (.blif, .eblif etc.) to represent vcc and gnd signal. And just briefly explain why they should be in the architecture database and their relationship with the vcc_cell and gnd_cell

[acomodi]

All right, I have added some extra comments around that, hopefully those help clarify a bit more on this.

[acomodi]

@tangxifan I believe this is ready to be merged, or do we want to wait for the fix in the strong sanitized tests?

[tangxifan]

@tangxifan I believe this is ready to be merged, or do we want to wait for the fix in the strong sanitized tests?

@acomodi I am fine to merge. The issue about the strong sanitized tests has been logged in #1934
@vaughnbetz Can you let me know if you are o.k. to waive the strong sanitized tests here?

[vaughnbetz]

I'm OK with waiving that. Not sure if there are any interchange format tests in the strong tests that get run through Sanitized Strong CI anyway. @acomodi: it would be good to add some basic interchange format tests to the strong run when the interchange code is stable enough (which might be now). We don't have to gate this PR on that though.

[acomodi]

Actually we might need to wait for the sanitizers tests to pass. I'll see if I can update the GCC version for kokoro directly in this PR.

@vaughnbetz, regarding the strong interchange test we'll need to wait to land some other PRs before we can start having integration testing to run all the way through the router. The code is actually ready, it just needs cleaning and some review passes.

[tangxifan]

@acomodi Thanks for the help on sanitized tests! CI is going green. I will approve once CI is finished.

[acomodi]

@tangxifan No problem, I managed to update compiler to GCC 9, as kokoro is running ubuntu 16.04, and for some reason I couldn't get the latest even though I added the ubuntu-latest repository to get the latest packages. Anyhow, all CI tests went green.