Noc description support and modelling in VPR #2010

Srivat97 · 2022-04-13T03:17:34Z

Extended the FPGA architecture description to now include NoC specific information. Then updated VPR to process the NoC information provided by the user and create an internal model. Finally, the graphics was updated to display the NoC to the user.

Description

libarchfpga

Updated the xml parser code to now include the additional tags used to describe the NoC. This information is then stored internally.

libvpr

Added a new class called NocRouter which represents a router within the NoC
Added a new class called NocLink which represents a link within the NoC (connections between routers)
Added a new class called NocStorage which represents the entire NoC (list of routers and links that together make up the NoC)
Created a new context called noc_ctx, this contains all the NoC related material so that it can be accessed globally
Added a new function called setup_noc that is called when the device is being created. This is responsible for building the NoC based on the information provided in the FPGA architecture description file. This includes creating all the routers and links.
Added a new set of command line options for the NoC.
Added a new button to display the NoC (draws the routers and links). Also can draw the NoC link usage as a heat map.

Related Issue

Motivation and Context

The eventual goal is to have a CAD tool that is capable of targeting FPGA architectures with an embedded NoC. But first we need to be able to get specifications about the NoC, such as its topology, location on the FPGA, and any design constraints (max link bandwidth, link latency, router latency and etc..). Once we have the NoC specifications, we need to model it so that we can ensure any steps taken in the CAD flow always meet the NoC design specifications. So the initial steps of getting the NoC information from the user and modelling it as done in this pull request.

How Has This Been Tested?

Unit tests were added to verify the newly developed functions
Additionally, sample FPGA architecture description files were created that included a description of an embedded NoC. VPR was run on the sample FPGA architecture description files with arbitrary designs and the NoC was displayed visually to ensure that the new feature worked correctly.

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

vaughnbetz · 2022-05-20T19:40:55Z

Should add an architecture file to test the overall flow works (augments the unit tests). It's OK if it's a simple architecture-in-progress.

vaughnbetz

Good overall, but some changes requested.
Most important: unit tests must test the real code (not a copy).

vaughnbetz · 2022-05-20T19:41:49Z

libs/libarchfpga/src/arch_util.cpp

@@ -204,6 +204,10 @@ void free_arch(t_arch* arch) {
    if (arch->clocks) {
        vtr::free(arch->clocks->clock_inf);
    }
+
+    if (arch->noc) {


Can skip this if, as delete on a NULL pointer is safe (defined to be a no-op).

vaughnbetz · 2022-05-20T19:47:15Z

libs/libarchfpga/src/physical_types.h

@@ -1773,6 +1773,29 @@ struct t_lut_element {
    }
 };

+/* Network-on-chip(NoC) Router data type used to identify 
+ * connections regarding individual routers in the network. */
+struct t_router {


Comment this is used when parsing the arch file (not long term storage). Comment each data member (what is it, any rules or usage).
Best to make Doxygen comments for this.

vaughnbetz · 2022-05-20T19:49:04Z

libs/libarchfpga/src/physical_types.h

+
+/* Network-on-chip(NoC) data type used to store the network 
+ * properties and used when builidng a dedicated on-chip network*/
+struct t_noc_inf {


Good to comment that we assume all links properties and router_latency are the same right now.

vaughnbetz · 2022-05-20T19:49:44Z

libs/libarchfpga/src/physical_types.h

+
+    std::vector<t_router> router_list;
+
+    std::string noc_router_tile_name;


Worth commenting that this must match some tile type name in the arch file.

vaughnbetz · 2022-05-20T19:54:14Z

libs/libarchfpga/src/read_xml_arch_file.cpp

+    pugiutil::expect_only_attributes(topology_tag, {}, loc_data);
+
+    // stores router information that includes the number of connections a router has within a given topology and also the number of times a router was declared in the arch file using the <router> tag
+    // in the datastructure below, the router id is the key and stored data is a pair, where the first element is describes the number router declarations and the second element describes the numbe router connections


is describes -> describes
the number -> the number of
numbe -> number of

Might also want to mention this is used only for error checking

vaughnbetz · 2022-05-20T20:41:09Z

vpr/src/draw/draw.cpp

@@ -187,13 +189,26 @@ static void set_block_text(GtkWidget* widget, gint /*response_id*/, gpointer /*d
 static void clip_routing_util(GtkWidget* widget, gint /*response_id*/, gpointer /*data*/);
 static void run_graphics_commands(std::string commands);

+// draw_noc helper functions
+static ezgl::rectangle get_noc_connection_marker_bbox(const t_logical_block_type_ptr noc_router_logical_block_type);


If these could go in a separate file (draw_noc.cpp) that would be good as draw.cpp is too big.
But if they're using file scope variables we can leave them. Could be good task for summer students to clean up.

Can I change some functions that I use externally from being static?

Sure. There's nothing too special about static; just a way to hide names inside a file if they aren't needed outside. If they are needed outside, feel free to remove the static and put their declarations in the .h file.

vaughnbetz · 2022-05-20T20:45:44Z

vpr/src/draw/draw_noc.cpp

+#    include "vpr_error.h"
+#    include "vtr_math.h"
+
+/**


Good to have a comment on what should go in this file.

vaughnbetz · 2022-05-20T20:48:32Z

vpr/src/draw/draw_noc.cpp

+        return result;
+    }
+
+    // if we are here then the line has a slope, so we use the cross product to determine the angle


I don't think this is really the cross product --> maybe you're figuring it out from the dot product? A temporary variable or two might help here too.

Yes, it was a dot product not a cross product. Changed and included comments.

vaughnbetz · 2022-05-20T20:51:56Z

vpr/src/draw/draw_noc.cpp

+                link_coords.start.x += noc_connection_marker_quarter_width;
+                link_coords.end.x += noc_connection_marker_quarter_width;
+
+                link_coords.start.y += noc_connection_marker_quarter_height;


Might be able to simplify by defining an offset (either x_off and y_off or an ezgl point offset) then you may be able to just always add and subtract the offset; the set up code would have figured out whether x_off, or y_off was 0 or negative etc.

Will leave as is for now. I'll think of a better solution and change later.

vaughnbetz · 2022-05-20T20:57:36Z

vpr/test/test_setup_noc.cpp

+
+/*Re-defining static functions in setup_noc.cpp that are tested here*/
+
+static void identify_and_store_noc_router_tile_positions(const DeviceGrid& device_grid, std::vector<t_noc_router_tile_position>& list_of_noc_router_tiles, std::string noc_router_tile_name) {


Better to make this non-static and test the real code, not a copy.

vaughnbetz · 2022-06-24T20:00:44Z

Infrstructure issue; CI should pass once I merge Alireza's change.
Good to get this merged soon; draw will have to be updated as it was heavily refactored.

…nclude a Network on chip

…ute and modified some function prototypes

…re file

…rd topologies such as 'mesh'

…architectyre file

… input. Also added a function that isolates noc router tiles from the device grid.

…d the object definition for a noc_link.

… router ids to the id system used for routers in the NoC

… in the arch description file

…tion

…e class that pre-allocates space for datastructures

…on the FPGA device

…m found the closest physical router to each logical router, instead of the other way around.

…lay the noc links or also the bandwidth usage of each link.

…e NoC (if it exsists)

…mody anything by making it a constant function

… links are seperated and not drawn on top of each other

…ique color to represent its usage.

…led" This reverts commit fa16da2.

… its build wouldn't fail

…ld failure when graphics isn't enabled

…ure when running the Unit tests during a push

…e inside the src directory was used. Not doing this resultedin the main.cpp file inside the test folder also being used and this caused a failure in the github CI. Also note that this how other projects are also setup.

…lass to print its information and also added functions to directly access routers and links in the NoC

…dicate that the id is supplied by the user. Also improved the documentation for the NocLink class.

…Also changed the class so that instead of functions that returned member variables of the NocRouter and NocLink class, new functions were added to return the NocRouter and NocLink themselves.

…o setup_noc.h,improved commenting in setup_noc.cpp, fixed some comments in noc_storage.h and finally removed re-declaration of static funtions in test_setup_noc.cpp

…ter latency from the noc context and moved it to the NocStorage class. Changed the echo function to include these noc properties as well. Changed all calls to those properties from using the noc context to using the NocStorage class.

…xygen comments to explain a few of the NoC drawing functions that were poorly commented before.

…w.cpp and draw.h

… to draw_toggle_functions.cpp. This was done to match the previous refactor changes.

…scription of their topology. Updated README inside /home/srivat97/Research_Project/work/vtr_flow/arch/ to help users find these newly added files.

Srivat97 · 2022-06-26T03:14:47Z

@vaughnbetz This PR should be ready to merge. Fixed all requested changes or made a comment to explain why I didn't make the change.

I also added some sample architecture files I used to test this new feature. They can be found under "$VTR_ROOT/vtr_flow/arch/noc". There are two architecture files in there, one has a mesh topology and the other has a star topology. I also updated the README found in "$VTR_ROOT/vtr_flow/" to help new users find these files and explain their purpose.

vaughnbetz · 2022-06-30T21:18:05Z

Looks good -- thanks!

Srivat97 requested a review from vaughnbetz April 13, 2022 03:17

github-actions bot added libarchfpga Library for handling FPGA Architecture descriptions VPR VPR FPGA Placement & Routing Tool labels Apr 13, 2022

Srivat97 force-pushed the noc_description_support branch from 5f40de0 to 7b96833 Compare April 13, 2022 17:03

vaughnbetz requested changes May 20, 2022

View reviewed changes

Srivat97 added 23 commits June 24, 2022 20:47

adding support to be able to process architecture descriptions that i…

b95d0cb

…nclude a Network on chip

changed function prototypes to improve readibility

5e1a04e

fixed a vector initialization bug, a bug related to parsing an attrib…

63837dd

…ute and modified some function prototypes

added support to verify a provided noc topology within the architectu…

f7c53df

…re file

added unit tests to the libarchfpga build condifuration

1677313

added functionality to the architecture file format to support standa…

bc8b1ff

…rd topologies such as 'mesh'

added unit tests to verify the new features that support nocs in the …

e18574e

…architectyre file

added new context variable to represent the NoC

ce066e7

added a new step in the vpr flow to setup noc model if provided as an…

69d9254

… input. Also added a function that isolates noc router tiles from the device grid.

removed unnecessary code used to test some data structures

319ab7e

created a noc router object

22fe34c

added a noc link object

10af173

changed link and router objects to noc_link and noc_router. Also adde…

9166443

…d the object definition for a noc_link.

Created a datastructure to represent the entire NoC

f923518

added functionality to the NoC datastructure to map the user provided…

7b92e2c

… router ids to the id system used for routers in the NoC

added messages to assertions

af5d8d2

ensured that the the NoC datastructure cannot be copied

4e7ff68

Added a check box in the graphics to control the display of the NoC

52d0be9

verified that connections in the NoC were not included more than once…

230d998

… in the arch description file

added support to fully build the NoC model from the arch file descrip…

ca06620

…tion

removed unnecessary destructors and added a function to the NocStorag…

9c80586

…e class that pre-allocates space for datastructures

added a check to catch the case where no physical router tiles exist …

f944cf1

…on the FPGA device

modified function that adds routers to noc model so that the algorith…

8612809

…m found the closest physical router to each logical router, instead of the other way around.

Srivat97 added 23 commits June 24, 2022 20:56

created a combo box that allows users to have the option to just disp…

b13676b

…lay the noc links or also the bandwidth usage of each link.

added a utility function that finds a parralel link of any link in th…

5aa5633

…e NoC (if it exsists)

made sure the new utility to find a parallel link in the noc did not …

c58f46f

…mody anything by making it a constant function

When displaying the NoC, the noc links are drawn so that any parallel…

b02a025

… links are seperated and not drawn on top of each other

Since the links don't overlap anymore, ensured that each link as a un…

64bc4d3

…ique color to represent its usage.

Improved code documentation (added comments)

905f69d

Revert "added a function to log the options used when the NoC is enab…

d81ec04

…led" This reverts commit fa16da2.

redefined static functions that were previously tested in test_vpr so…

b9069cb

… its build wouldn't fail

added directives to the draw noc related code so that there is no bui…

802086d

…ld failure when graphics isn't enabled

fixed an issue with the NocStorage class unit test that caused a fail…

1284050

…ure when running the Unit tests during a push

fixed compiler issue when including catch test framework header

83ca73d

making requested PR changes

65bd38d

making requested PR changes. Added a new function to the NocStorage c…

3c23389

…lass to print its information and also added functions to directly access routers and links in the NoC

added renamed router id variable to user_id, this was done to help in…

0058da5

…dicate that the id is supplied by the user. Also improved the documentation for the NocLink class.

changed the documentation for the NocStorage class to doxygen style. …

37bb913

…Also changed the class so that instead of functions that returned member variables of the NocRouter and NocLink class, new functions were added to return the NocRouter and NocLink themselves.

Removed static functions in setup_noc.cpp. Added top level comments t…

64540ca

…o setup_noc.h,improved commenting in setup_noc.cpp, fixed some comments in noc_storage.h and finally removed re-declaration of static funtions in test_setup_noc.cpp

removed NoC drawing functions from draw.cpp to draw_noc.cpp. Added do…

e2e2c51

…xygen comments to explain a few of the NoC drawing functions that were poorly commented before.

added doxygen commenting tp draw_noc and also removed uneccesarry const

cfe6200

Changed some documentayion and improved code readibility

18b3746

fixed issues associated with bringing in changes from refactoring dra…

95dae7f

…w.cpp and draw.h

moved the call back function for the NoC display button from draw.cpp…

9b81271

… to draw_toggle_functions.cpp. This was done to match the previous refactor changes.

Srivat97 force-pushed the noc_description_support branch from 33632a5 to 9b81271 Compare June 25, 2022 20:04

added sample architecture files that include an embedded NoC and a de…

421e4be

…scription of their topology. Updated README inside /home/srivat97/Research_Project/work/vtr_flow/arch/ to help users find these newly added files.

making formatting changes

7c841f5

vaughnbetz merged commit b30b9fa into master Jun 30, 2022

vaughnbetz deleted the noc_description_support branch June 30, 2022 21:18


		std::vector<t_router> router_list;

		std::string noc_router_tile_name;


		/Re-defining static functions in setup_noc.cpp that are tested here/

		static void identify_and_store_noc_router_tile_positions(const DeviceGrid& device_grid, std::vector<t_noc_router_tile_position>& list_of_noc_router_tiles, std::string noc_router_tile_name) {

Noc description support and modelling in VPR #2010

Noc description support and modelling in VPR #2010

Uh oh!

Conversation

Srivat97 commented Apr 13, 2022

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

vaughnbetz commented May 20, 2022

Uh oh!

vaughnbetz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vaughnbetz commented Jun 24, 2022

Uh oh!

Srivat97 commented Jun 26, 2022

Uh oh!

vaughnbetz commented Jun 30, 2022

Uh oh!

Uh oh!