FPGA Interchange: improve arch reader #1937

acomodi · 2021-12-15T14:50:51Z

Description

This PR is to improve and add missing features to be able to correctly read the interchange device, and place and route a design using the automatically generated RR graph.

This is true for all the currently available interchange architectures, namely xilinx series7, lattice nexus and the basic test-architecture.

Given that the RR graph is auto-generated, the route file will be invalid, but we can start adding regression tests to verify that the whole flow is correct.

Motivation and Context

This is in the contect of adding support to the FPGA interchange format

How Has This Been Tested?

Unit C++ 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

acomodi · 2021-12-15T16:14:48Z

@tangxifan @vaughnbetz I opted to open a bigger PR to include all the changes necessary to have a LUT design to be placed and routed with the interchange format.

The diff is quite big, but all the major changes are only corresponding to the interchange arch reader, to which I have added some high level comments.

tangxifan

@acomodi Changes make sense to me. Major comments are on the API comments and code clean-up. Feel free to tell me if there is anything confusing.

libs/libarchfpga/src/arch_util.h

vpr/src/base/SetupVPR.cpp

vpr/src/pack/pb_type_graph.cpp

tangxifan · 2022-01-20T17:57:39Z

libs/libvtrutil/src/vtr_util.h

@@ -118,6 +118,8 @@ void uniquify(Container container) {

 int get_pid();

+char* stringf(const char* format, ...);


Is it possible add comments on this API?

This API is actually not really required. The fact is that IMO it is better to make wider use of std::string and C++ standards to build strings. I have changed the interchange reader to avoid using this string formatter API

libs/libarchfpga/src/arch_util.h

This commit also reworks the way the name is assigned to the null types Signed-off-by: Alessandro Comodi <[email protected]>

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

These workarounds can be removed once the RR graph generation is in place Signed-off-by: Alessandro Comodi <[email protected]>

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

Signed-off-by: Maciej Dudek <[email protected]>

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

Moreover, this commit reduces code verbosity to get directions of pins Signed-off-by: Alessandro Comodi <[email protected]>

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

Signed-off-by: Maciej Kurc <[email protected]>

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

tangxifan

@acomodi Thanks for the effort. Most comments have been well addressed. Just some minor details to cover.

libs/libarchfpga/src/arch_util.h

libs/libarchfpga/src/read_xml_arch_file.cpp

tangxifan · 2022-01-21T18:31:46Z

libs/libarchfpga/src/physical_types.h

@@ -653,6 +653,9 @@ struct t_physical_tile_type {

    // Does this t_physical_tile_type contain an outpad?
    bool is_output_type = false;
+
+    // Is this t_physical_tile_type an empty type?
+    bool is_empty = false;


When we will refactor the data structure, I think I will not keep a boolean variable. Currently, as far as I see, to identify an empty physical_tile, there are multiple ways. I see in some codes, the name is used as a reference, while in other codes, the index is used as a reference. That will create issues. That is why we need an API is_empty() to provide a general reference. I do not have a strong opinion whether to use name or index. But we should use an API so that developers have a standard way to follow.

For example:

bool is_empty() { return std::string(name)== EMPTY_BLOCK_NAME; }

I am open to discussion. Let me know what you think.

Right, I have actually implemented this change in 162ca07. Also, I think that for now, the Interchange can also make use of the default name for the EMPTY blocks, but it is worth noticing that, if this can be a customizable parameter, relying on its name to verify whether a block is empty could not be the best solution.

I am thinking that maybe we can use the same approach as for the architecture name?

@acomodi The approach for the architecture name makes sense to me. We follow this way.

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

acomodi · 2022-01-25T08:43:17Z

All CI checks passed. Merging

This was referenced Dec 15, 2021

WIP: libs: arch: interchange: cell bel mapping implementation #1927

Closed

libs: arch: interchange: add logical block and physical tiles support #1926

Closed

acomodi requested review from tangxifan and vaughnbetz and removed request for vaughnbetz December 15, 2021 15:03

github-actions bot added libarchfpga Library for handling FPGA Architecture descriptions VPR VPR FPGA Placement & Routing Tool labels Dec 15, 2021

acomodi force-pushed the acom/fpga-interchange-improve-arch-reading branch from 64cb803 to 7a1a6fa Compare December 21, 2021 10:32

github-actions bot added the libvtrutil label Dec 21, 2021

acomodi mentioned this pull request Dec 21, 2021

LUT element support antmicro/vtr-verilog-to-routing#7

Open

acomodi force-pushed the acom/fpga-interchange-improve-arch-reading branch from 7a1a6fa to 21728dd Compare December 22, 2021 16:18

acomodi force-pushed the acom/fpga-interchange-improve-arch-reading branch from 21728dd to f439cd4 Compare January 13, 2022 15:57

tangxifan requested changes Jan 20, 2022

View reviewed changes

github-actions bot removed the libvtrutil label Jan 21, 2022

acomodi requested a review from tangxifan January 21, 2022 10:43

acomodi and others added 13 commits January 21, 2022 12:04

libs: arch: move common functions to util and check source files

e7f55d3

This commit also reworks the way the name is assigned to the null types Signed-off-by: Alessandro Comodi <[email protected]>

libs: arch: interchange: add logical blocks and physical tile parsing

bd20726

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

vpr: tests: improve interchange tests

3e969ea

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

libs: arch: interchange: improve bel to cell mapping

9037dca

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

libs: arch: fix multiple sites and rr graph gen

a9444cd

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

libs: arch: interchange: fix IO pads reading

b8a5a88

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

libs: arch: interchange: add workarounds for RR graph generation

a06b406

These workarounds can be removed once the RR graph generation is in place Signed-off-by: Alessandro Comodi <[email protected]>

vpr: test: interchange: fix grid width and height

98a1718

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

vpr: base: remove cStr() casts from interchange netlist reader

5dca717

Signed-off-by: Maciej Dudek <[email protected]>

libs: arch: interchange: fix and enhance to start reading in xc7 arch

06c8abd

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

libs: arch: interchange: allow cascading routing bels

5d17366

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

vpr: pack: allow pins to have name starting with integer

749b402

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

libs: arch: interchange : allow inout ports in bels connected to pads

9c7fb88

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

acomodi and others added 7 commits January 21, 2022 12:04

vpr: base: interchange: netlist: add c-style init and fixed bug

b7c962d

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

libs: arch: interchange: add pack pattern propagation

a97a3e9

Moreover, this commit reduces code verbosity to get directions of pins Signed-off-by: Alessandro Comodi <[email protected]>

libs: arch: interchange: clean and add in-code comments

c513f9c

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

vpr: test: adjust grid layout test as NULL tile is not in the grid

a017e53

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

Added reading LUT elements from FPGA interchange

21ee47c

Signed-off-by: Maciej Kurc <[email protected]>

libs: arch: interchange: fix luts that have additional pins

857e7ce

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

interchange: address review comments

821bff9

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

acomodi force-pushed the acom/fpga-interchange-improve-arch-reading branch from 77f2ea9 to 821bff9 Compare January 21, 2022 11:04

tangxifan requested changes Jan 21, 2022

View reviewed changes

acomodi added 2 commits January 24, 2022 11:26

libs: arch: add is_empty function to phys/log types

162ca07

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

vpr: test: fix interchange test

450540c

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

tangxifan approved these changes Jan 24, 2022

View reviewed changes

acomodi merged commit 44f9683 into verilog-to-routing:master Jan 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

FPGA Interchange: improve arch reader #1937

FPGA Interchange: improve arch reader #1937

Uh oh!

acomodi commented Dec 15, 2021

Uh oh!

acomodi commented Dec 15, 2021

Uh oh!

tangxifan left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tangxifan Jan 20, 2022

Uh oh!

acomodi Jan 21, 2022

Uh oh!

Uh oh!

tangxifan left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tangxifan Jan 21, 2022

Uh oh!

acomodi Jan 24, 2022

Uh oh!

tangxifan Jan 24, 2022

Uh oh!

acomodi commented Jan 25, 2022

Uh oh!

Uh oh!

		@@ -118,6 +118,8 @@ void uniquify(Container container) {

		int get_pid();

		char* stringf(const char* format, ...);

FPGA Interchange: improve arch reader #1937

FPGA Interchange: improve arch reader #1937

Uh oh!

Conversation

acomodi commented Dec 15, 2021

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

acomodi commented Dec 15, 2021

Uh oh!

tangxifan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tangxifan Jan 20, 2022

Choose a reason for hiding this comment

Uh oh!

acomodi Jan 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tangxifan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tangxifan Jan 21, 2022

Choose a reason for hiding this comment

Uh oh!

acomodi Jan 24, 2022

Choose a reason for hiding this comment

Uh oh!

tangxifan Jan 24, 2022

Choose a reason for hiding this comment

Uh oh!

acomodi commented Jan 25, 2022

Uh oh!

Uh oh!