Now rr_graph -related source files are placed in a separated library `librrgraph` #1972

tangxifan · 2022-02-11T04:06:22Z

Description

This PR creates a C++ library of routing resource graph -related data structures.

Related Issue

Address issue #1868

Motivation and Context

Previously, data structures are tightly integrated with the VPR source codes (part of the router)
However, we see a strong motivation in creating a C++ library for the routing resource graph data structures.
This is because the routing resource graph is part of device modeling which should be decoupled with any EDA engines.
The benefits include

Decouple routing resource graph data structure from routers as well as other codes of VPR
Easier to profiling the data structure by runtime and peak memory usage. So that we can improve the data structure easily

How Has This Been Tested?

This PR includes the following changes

Move rr_graph data structures from vpr/src to libs/librrgraph
Clean-up all the APIs by removing all the dependencies on VPR global variables
Move all the data types for routing resource graph to a separated header file rr_node_types.h. (Name of the file can be changed)
Change all the error API from VPR_FATAL_ERROR to VTR_LOG_ERROR
Change documentation to track the code comments in libs/librrgraph

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

…ight rework

… to RRGraphView

…_graph_storage to RRGraphBuilder

tangxifan · 2022-02-11T06:40:45Z

@umariqbal-rs Can you look into the doc errors:

Also if CI is green, can you run QoR tests? This time, it is better to test on different machines and see if there are any degradation. We are very sensitive to performance degradation in this PR.

umariqbal-rs · 2022-02-11T09:40:39Z

@tangxifan
Sure I will do them.

mithro · 2022-02-11T17:35:31Z

Would be good to get someone from Antmicro like @acomodi or @kgugala to review this change to understand if / how this will affect SymbiFlow.

tangxifan · 2022-02-11T18:07:49Z

@mithro Yes. It will be great. I have requested their review.

umariqbal-rs · 2022-02-14T06:15:56Z

@tangxifan
I think there is an issue in Cmake can you Please have a look?

tangxifan · 2022-02-14T06:18:47Z

@tangxifan I think there is an issue in Cmake can you Please have a look?

Emm. Weird that CI does not capture it. Can you show your CMake version?
Also comment out LINE11. It should work then.

umariqbal-rs · 2022-02-14T06:22:58Z

@tangxifan
cmake version 3.16.2

umariqbal-rs · 2022-02-14T06:28:55Z

@tangxifan I think there is an issue in Cmake can you Please have a look?

Emm. Weird that CI does not capture it. Can you show your CMake version? Also comment out LINE11. It should work then.

Thanks It works after commenting line 11.

acomodi

LGTM, I think that SymbiFlow won't be affected by these changes, also confirmed by the passing symbiflow tests

acomodi · 2022-02-14T09:02:30Z

libs/librrgraph/src/base/rr_graph_storage.h

-    edge_idx_range configurable_edges(const RRNodeId& id) const {
-        return vtr::make_range(edge_idx_iterator(0), edge_idx_iterator(num_edges(id) - num_non_configurable_edges(id)));
+
+    edge_idx_range configurable_edges(const RRNodeId& id, const vtr::vector<RRSwitchId, t_rr_switch_inf>& rr_switches) const {
+        return vtr::make_range(edge_idx_iterator(0), edge_idx_iterator(num_edges(id) - num_non_configurable_edges(id, rr_switches)));
    }
-    edge_idx_range non_configurable_edges(const RRNodeId& id) const {
-        return vtr::make_range(edge_idx_iterator(num_edges(id) - num_non_configurable_edges(id)), edge_idx_iterator(num_edges(id)));
+    edge_idx_range non_configurable_edges(const RRNodeId& id, const vtr::vector<RRSwitchId, t_rr_switch_inf>& rr_switches) const {
+        return vtr::make_range(edge_idx_iterator(num_edges(id) - num_non_configurable_edges(id, rr_switches)), edge_idx_iterator(num_edges(id)));


I suppose that the call to the edge partitioning in configurable/non-configurable edges has already been executed before calling these functions. Even though this change does not modify the previous behavior, it may be worth adding assertions here though, to check that the partitioned_ flag has been set, and put it under an assert_safe if we want to avoid any kind of overhead (not sure how often these are called).

@acomodi Agree. I just double check the APIs.
When calling num_configurable_edges() and num_non_configurable_edges(), the internal flags are checked to ensure edges are already partitioned. It is not exactly the partition_ flag but something equivalent (assigned during partition_edges() API call).
Let me know if this is sufficient or not.

vtr-verilog-to-routing/libs/librrgraph/src/base/rr_graph_storage.cpp

Lines 515 to 516 in f91b728

t_edge_size t_rr_graph_storage::num_configurable_edges(RRNodeId id, const vtr::vector<RRSwitchId, t_rr_switch_inf>& rr_switches) const {

VTR_ASSERT(!node_first_edge_.empty() && remapped_edges_);

vtr-verilog-to-routing/libs/librrgraph/src/base/rr_graph_storage.cpp

Lines 491 to 513 in f91b728

void t_rr_graph_storage::partition_edges(const vtr::vector<RRSwitchId, t_rr_switch_inf>& rr_switches) {

if (partitioned_) {

return;

}

edges_read_ = true;

VTR_ASSERT(remapped_edges_);

// This sort ensures two things:

// - Edges are stored in ascending source node order. This is required

// by assign_first_edges()

// - Edges within a source node have the configurable edges before the

// non-configurable edges.

std::sort(

edge_sort_iterator(this, 0),

edge_sort_iterator(this, edge_src_node_.size()),

edge_compare_src_node_and_configurable_first(rr_switches));

partitioned_ = true;

assign_first_edges();

VTR_ASSERT_SAFE(validate(rr_switches));

}

umariqbal-rs · 2022-02-17T04:54:31Z

@tangxifan
QoR tests are completed with current Master and quick summary is given below. QoR comparison files are also attached.
vtr_reg_qor_chain_depop test:
Flow run time 2.91% on average.
critical path delay 0% on average.
Peak memory usage 0.06% on average
titan_quick_qor test:
flow run time 3.29% on average.
critical path delay 0% on average.
Peak memory usage -0.13% on average
titan_quick_qor.xlsx
vtr_reg_qor_chain_depop.xlsx

tangxifan · 2022-02-17T06:16:53Z

@umariqbal-rs Can you upload the spreadsheets? Is it on a machine or two machines?

umariqbal-rs · 2022-02-17T07:06:18Z

@umariqbal-rs Can you upload the spreadsheets? Is it on a machine or two machines?

I already did .
We will upload tomorrow the second one.

tangxifan · 2022-02-17T18:50:13Z

@umariqbal-rs We have discussed the PR today with @vaughnbetz
He thinks the PR is good in terms of code changes and QoR tests are acceptable.
Please

Fix the broken links in the documentation
Upload the QoR tests from the 2nd machine

Then we can merge this PR.

vaughnbetz · 2022-02-17T19:22:07Z

libs/librrgraph/src/base/rr_node_types.h

+typedef enum e_rr_type : unsigned char {
+    SOURCE = 0, ///<A dummy node that is a logical output within a block -- i.e., the gate that generates a signal.
+    SINK,       ///<A dummy node that is a logical input within a block -- i.e. the gate that needs a signal.
+    IPIN,


Suggest adding comment for each enum.
IPIN: input pin to a block
OPIN: output pin of a block
CHANX: x-directed routing wire, or an x-directed segment of a channel for global routing
CHANY: y-directed routing wire, or a y-directed segment of a channel for global routing

vaughnbetz · 2022-02-17T19:28:25Z

libs/librrgraph/src/base/rr_node_types.h

+    float C;
+};
+
+//[0..num_rr_types-1][0..grid_width-1][0..grid_height-1][0..NUM_SIDES-1][0..max_ptc-1]


Should comment what this is for (fast lookups of an rr-node given an (rr_type, x, y, and the side).

vaughnbetz · 2022-02-17T19:31:18Z

Code looks good; I had a few minor suggestions on commenting above; can address after merging if you prefer.

tangxifan · 2022-02-17T20:11:22Z

@vaughnbetz No worries

@umariqbal-rs Please refine code comments as suggested.

behzadmehmood-rs · 2022-02-22T05:01:28Z

@tangxifan
QoR tests are completed with current Master on second server and quick summary is given below. QoR comparison files are also attached.
vtr_reg_qor_chain_depop test:
Flow run time -0.38% on average.
critical path delay 0% on average.
Peak memory usage -0.03% on average
titan_quick_qor test:
flow run time 0.11% on average.
critical path delay 0% on average.
Peak memory usage 0% on average
titan_quick_qor_master.xlsx
vtr_reg_qor_chain_depop.xlsx

tangxifan · 2022-02-22T05:39:06Z

Thanks @behzadmehmood-rs for the updates. QoR looks good to me.

@vaughnbetz I have updated the comments and fixed the broken links in documentation on the RRGraph APIs.
I propose to merge as QoR results are also clean.
If you have any further comments. Feel free to shot, we will address it in follow-up pull requests.

tangxifan added 16 commits February 10, 2022 11:21

[Lib] Moving rr_graph data structure to a library

70e01cc

[Lib] Add Cmakelist for librrgraph

4a547e1

[Lib] Add librrgraph to higher level CMakelist

2e55317

[VPR] Add librrgraph to the dependency of VPR

eddcc08

[lib] Disable unit tests for now; Focus on compilation errors

f98a93e

[Lib] Debugging compilation errors

cd5f9ee

[Lib] Adapt sources for dependencies on data structures

c7a979d

[Lib] Detecting some low-level APIs require global variables; Need sl…

0f1c3cd

…ight rework

[Lib] Now move edge_is_configurable() and validate() API from rr_node…

b6840e5

… to RRGraphView

[Lib] Move validate() function and partition_edges() function from rr…

56165c6

…_graph_storage to RRGraphBuilder

[Lib] Resolving dependency problem for rr_node_impl files

6631527

[Lib] Now RC data is referenced in the RRGraphView

6e431d7

[Lib] Fixed all the errors in the librrgraph

6a61e7a

[Lib] Fixing type bugs

dac9b10

[Lib] Fix compiler warnings

4d62124

[Lib] Add missing files

8b830b5

tangxifan changed the title ~~Now rr_graph -related source files are placed in a separated library librrgraph~~ [WIP] Now rr_graph -related source files are placed in a separated library librrgraph Feb 11, 2022

github-actions bot added the VPR VPR FPGA Placement & Routing Tool label Feb 11, 2022

[VPR] Code format

f91b728

tangxifan mentioned this pull request Feb 11, 2022

Dashboard for RRGraph API refactoring status #1868

Open

44 tasks

tangxifan requested review from acomodi, kgugala and vaughnbetz February 11, 2022 18:07

acomodi reviewed Feb 14, 2022

View reviewed changes

vaughnbetz reviewed Feb 17, 2022

View reviewed changes

tangxifan added 4 commits February 18, 2022 16:16

[Doc] Fix the broken links in API doc

c583013

[Lib] Add code comments

a4baac7

[Doc] Fixing the broken links in API doc

f30c62f

[Doc] Typo

04f0a02

tangxifan changed the title ~~[WIP] Now rr_graph -related source files are placed in a separated library librrgraph~~ Now rr_graph -related source files are placed in a separated library librrgraph Feb 19, 2022

tangxifan merged commit 8e7efe5 into master Feb 22, 2022

tangxifan deleted the librrgraph branch February 22, 2022 05:39

tangxifan restored the librrgraph branch February 22, 2022 05:39

tangxifan deleted the librrgraph branch February 22, 2022 05:39

tangxifan mentioned this pull request Jul 28, 2022

Move rr_graph_reader & rr_graph_writer to librrgraph. #2101

Merged

7 tasks

	t_edge_size t_rr_graph_storage::num_configurable_edges(RRNodeId id, const vtr::vector<RRSwitchId, t_rr_switch_inf>& rr_switches) const {
	VTR_ASSERT(!node_first_edge_.empty() && remapped_edges_);

	void t_rr_graph_storage::partition_edges(const vtr::vector<RRSwitchId, t_rr_switch_inf>& rr_switches) {
	if (partitioned_) {
	return;
	}

	edges_read_ = true;
	VTR_ASSERT(remapped_edges_);
	// This sort ensures two things:
	// - Edges are stored in ascending source node order. This is required
	// by assign_first_edges()
	// - Edges within a source node have the configurable edges before the
	// non-configurable edges.
	std::sort(
	edge_sort_iterator(this, 0),
	edge_sort_iterator(this, edge_src_node_.size()),
	edge_compare_src_node_and_configurable_first(rr_switches));

	partitioned_ = true;

	assign_first_edges();

	VTR_ASSERT_SAFE(validate(rr_switches));
	}

Now rr_graph -related source files are placed in a separated library librrgraph #1972

Now rr_graph -related source files are placed in a separated library librrgraph #1972

Uh oh!

Conversation

tangxifan commented Feb 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

tangxifan commented Feb 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

umariqbal-rs commented Feb 11, 2022

Uh oh!

mithro commented Feb 11, 2022

Uh oh!

tangxifan commented Feb 11, 2022

Uh oh!

umariqbal-rs commented Feb 14, 2022

Uh oh!

tangxifan commented Feb 14, 2022

Uh oh!

umariqbal-rs commented Feb 14, 2022

Uh oh!

umariqbal-rs commented Feb 14, 2022

Uh oh!

acomodi left a comment

Choose a reason for hiding this comment

Uh oh!

acomodi Feb 14, 2022

Choose a reason for hiding this comment

Uh oh!

tangxifan Feb 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

umariqbal-rs commented Feb 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tangxifan commented Feb 17, 2022

Uh oh!

umariqbal-rs commented Feb 17, 2022

Uh oh!

tangxifan commented Feb 17, 2022

Uh oh!

vaughnbetz Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

vaughnbetz Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

vaughnbetz commented Feb 17, 2022

Uh oh!

tangxifan commented Feb 17, 2022

Uh oh!

behzadmehmood-rs commented Feb 22, 2022

Uh oh!

tangxifan commented Feb 22, 2022

Uh oh!

Uh oh!

Now rr_graph -related source files are placed in a separated library `librrgraph` #1972

Now rr_graph -related source files are placed in a separated library `librrgraph` #1972

tangxifan commented Feb 11, 2022 •

edited

Loading

tangxifan commented Feb 11, 2022 •

edited

Loading

tangxifan Feb 14, 2022 •

edited

Loading

umariqbal-rs commented Feb 17, 2022 •

edited

Loading