RRGraphView edges(), num_edges(), configurable_edges(), non_configurable_edges(), num_configurable_edges(), num_non_configurable_edges() Implementation #1917

behzadmehmood-rs · 2021-11-18T11:44:34Z

Description

In this PR, I am implementing RRGraphView::edges(), RRGraphView::num_edges(), RRGraphView::configurable_edges(), RRGraphView::non_configurable_edges(), RRGraphView::num_configurable_edges() and RRGraphView::num_non_configurable_edges() throughout VTR. Each device_ctx.rr_nodes[node].edges() call has been replaced with rr_graph.num_edges(RRNodeId(node)). The same pattern was followed for the other functions in this PR. In order to do this, I followed a pattern similar to that in the last PR.

Related Issue

Motivation and Context

These changes are a continuation of the RRGraphView API implementation effort.

How Has This Been Tested?

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

vpr/src/device/rr_graph_view.h

vpr/src/route/rr_node.cpp

tangxifan · 2021-11-19T06:33:07Z

vpr/src/route/rr_node_impl.h

@@ -90,10 +90,6 @@ inline t_edge_size t_rr_node::num_non_configurable_edges() const {
    return storage_->num_non_configurable_edges(id_);
 }

-inline t_edge_size t_rr_node::num_configurable_edges() const {


Good. Can you double check this removal in the #1916?

tangxifan

@behzadmehmood-rs Please check my comments on the rr_graph_uxsdcxx_serializer.h
This PR is almost there.
After applying these comments, please wait until the CI is green before starting QoR tests.

vpr/src/route/rr_graph.cpp

vpr/src/route/rr_graph_uxsdcxx_serializer.h

behzadmehmood-rs · 2021-12-10T06:04:44Z

QoR tests are complete and quick summary is given below. QoR comparison files are also attached.
vtr_reg_qor_chain_depop test:
Flow run time 12.17% on average.
critical path delay 0% on average.
Peak memory usage -0.1% on average
titan_quick_qor test:
flow run time 33.31% on average.
critical path delay 0% on average.
Peak memory usage -0.01% on average
titan_quick_qor.xlsx
vtr_reg_qor_chain_depop.xlsx

tangxifan · 2021-12-10T06:22:13Z

@behzadmehmood-rs Thanks for the effort. It looks fine. Can you compare to current master also (see if the runtime overhead is indeed due to this PR or not)?

behzadmehmood-rs · 2021-12-10T06:24:25Z

@behzadmehmood-rs Thanks for the effort. It looks fine. Can you compare to current master also (see if the runtime overhead is indeed due to this PR or not)?

Sure, will do that.

behzadmehmood-rs · 2021-12-13T04:55:05Z

@behzadmehmood-rs Thanks for the effort. It looks fine. Can you compare to current master also (see if the runtime overhead is indeed due to this PR or not)?

QoR tests for master branch are complete and quick summary is given below. QoR comparison files are also attached.
vtr_reg_qor_chain_depop test:
Flow run time 12.24% on average.
critical path delay 0% on average.
Peak memory usage -0.02% on average
titan_quick_qor test:
flow run time 21.61% on average.
critical path delay 0% on average.
Peak memory usage -0.01% on average

titan_quick_qor.xlsx
vtr_reg_qor_chain_depop.xlsx

tangxifan

@behzadmehmood-rs I see the runtime overhead. They are mainly from placement and routing. It indicates that our modification slows down the algorithm. I suspect the use of RRGraphView in rr_node.cpp is the source. Would you mind revert it to previous codes for validate()? See if this is the cause or not.

behzadmehmood-rs · 2021-12-13T07:03:24Z

@behzadmehmood-rs I see the runtime overhead. They are mainly from placement and routing. It indicates that our modification slows down the algorithm. I suspect the use of RRGraphView in rr_node.cpp is the source. Would you mind revert it to previous codes for validate()? See if this is the cause or not.

Sure, will do that.

behzadmehmood-rs · 2021-12-14T04:24:10Z

@behzadmehmood-rs I see the runtime overhead. They are mainly from placement and routing. It indicates that our modification slows down the algorithm. I suspect the use of RRGraphView in rr_node.cpp is the source. Would you mind revert it to previous codes for validate()? See if this is the cause or not.

QoR tests after reverting changes for validate() are complete and quick summary is given below. QoR comparison files are also attached.
vtr_reg_qor_chain_depop test:
Flow run time 9.93% on average.
critical path delay 0% on average.
Peak memory usage -0.08% on average
titan_quick_qor test:
flow run time 20.46% on average.
critical path delay 0% on average.
Peak memory usage -0.02% on average

titan_quick_qor.xlsx
vtr_reg_qor_chain_depop.xlsx

behzadmehmood-rs · 2021-12-14T04:26:42Z

@tangxifan I have compared results after reverting changes for validate() and uploaded the files. Should I push changes to the branch?

tangxifan · 2021-12-14T04:30:35Z

@tangxifan I have compared results after reverting changes for validate() and uploaded the files. Should I push changes to the branch?

Is it compared to the current master or a branch which is created before the whole refactoring effort?

behzadmehmood-rs · 2021-12-14T04:35:07Z

@tangxifan I have compared results after reverting changes for validate() and uploaded the files. Should I push changes to the branch?

Is it compared to the current master or a branch which is created before the whole refactoring effort?

It is compared with test_sanity_basic branch

tangxifan · 2021-12-14T04:36:15Z

@behzadmehmood-rs Sure. Can you let me know any changes on the runtime when reverted?

behzadmehmood-rs · 2021-12-14T04:44:03Z

@behzadmehmood-rs Sure. Can you let me know any changes on the runtime when reverted?

@tangxifan following changes were made.

tangxifan · 2021-12-14T04:57:05Z

@behzadmehmood-rs Can you show me the changes on runtime? I am cool with the code changes.

behzadmehmood-rs · 2021-12-14T05:01:53Z

@tangxifan I have compared results after reverting changes for validate() and uploaded the files. Should I push changes to the branch?

Is it compared to the current master or a branch which is created before the whole refactoring effort?

QoR comparison results with master branch and the current branch after reverting cahnges for validate() are given below. QoR comparison files are also attached.
vtr_reg_qor_chain_depop test:
Flow run time -2.06% on average.
critical path delay 0% on average.
Peak memory usage -0.06% on average
titan_quick_qor test:
flow run time -0.95% on average.
critical path delay 0% on average.
Peak memory usage -0.02% on average

titan_quick_qor_master.xlsx
vtr_reg_qor_chain_depop_master.xlsx

tangxifan · 2021-12-14T05:04:15Z

This looks good to me now. Please revert the changes. Thanks a lot!

behzadmehmood-rs · 2021-12-14T05:05:21Z

This looks good to me now. Please revert the changes. Thanks a lot!

I have pushed reverted changes.

tangxifan · 2021-12-14T05:08:31Z

@behzadmehmood-rs No worries. Can you fix the errors in CI？
Waive red flag on the strong sanitized test.
Once CI is green, I will propose to merge.

behzadmehmood-rs · 2021-12-14T10:54:36Z

@behzadmehmood-rs No worries. Can you fix the errors in CI？ Waive red flag on the strong sanitized test. Once CI is green, I will propose to merge.

@tangxifan kindly review.

tangxifan · 2021-12-14T18:34:05Z

@behzadmehmood-rs Look good to me. I propose to merge this week. Good work.

tangxifan · 2021-12-14T22:08:15Z

@vaughnbetz I have reviewed this PR several times. QoR test is clean. I propose to merge. If you see any issue, feel free to comment. I will actively work on it.

vpr/src/device/rr_graph_view.h

Added API for num_configurable_edges

91eb4b5

github-actions bot added the VPR VPR FPGA Placement & Routing Tool label Nov 18, 2021

behzadmehmood-rs changed the title ~~Added API for num_configurable_edges~~ RRGraphView::num_configurable_edges() Implementation Nov 18, 2021

tangxifan mentioned this pull request Nov 18, 2021

Dashboard for RRGraph API refactoring status #1868

Open

44 tasks

behzadmehmood-rs added 2 commits November 19, 2021 09:14

Updated formatting for rr_node.cpp

f0210f1

Removed num_configurable_edges function from rr_node_impl.h

0ae6368

tangxifan requested changes Nov 19, 2021

View reviewed changes

vpr/src/device/rr_graph_view.h Show resolved Hide resolved

vpr/src/route/rr_node.cpp Outdated Show resolved Hide resolved

behzadmehmood-rs changed the title ~~RRGraphView::num_configurable_edges() Implementation~~ RRGraphView configurable_edges(), non_configurable_edges(), num_configurable_edges(), num_non_configurable_edges() Implementation Nov 19, 2021

Added comment for num_configurable_edges

1f22e38

tangxifan reviewed Nov 19, 2021

View reviewed changes

behzadmehmood-rs added 6 commits November 22, 2021 12:55

API added for edges()

cad8da6

Added comment for edges() API

cac4434

Removed edges() from rr_node_impl.h

6164123

Adding num_edges() API

25ec80c

Added num_edges() API

83fbf86

removed num_edges() from rr_node_impl.h

8d77256

behzadmehmood-rs marked this pull request as ready for review November 25, 2021 04:45

behzadmehmood-rs marked this pull request as draft November 25, 2021 04:53

behzadmehmood-rs and others added 4 commits November 25, 2021 10:17

Merge branch 'master' into api_num_configurable_edges

5527733

num_non_configurable_edges() API added

dc4a62f

configurable_edges() API added

5795252

Added non_configurable_edges() API

46ac27e

behzadmehmood-rs added 2 commits December 2, 2021 11:26

Removed globals from rr_graph_uxsdcxx_serializer.h

6d14f83

Updated formatting for rr_graph_uxsdcxx_serializer.h

8efc261

behzadmehmood-rs mentioned this pull request Dec 6, 2021

RRGraphView edge_sink_node()/edge_switch() Implementation #1930

Merged

7 tasks

tangxifan requested changes Dec 6, 2021

View reviewed changes

vpr/src/route/rr_graph.cpp Outdated Show resolved Hide resolved

vpr/src/route/rr_graph_uxsdcxx_serializer.h Outdated Show resolved Hide resolved

behzadmehmood-rs and others added 3 commits December 7, 2021 10:02

Typo fixed in rr_graph.cpp

88c5785

A cached version of RRGraphView added in rr_graph_uxsdcxx_serializer.h.

09bbfab

formatting has been set

95ca4bd

tangxifan marked this pull request as ready for review December 10, 2021 06:22

behzadmehmood-rs requested a review from tangxifan December 13, 2021 05:38

tangxifan reviewed Dec 13, 2021

View reviewed changes

Changes reverted for validate()

6b834d8

Formatting updated

a4a5abf

behzadmehmood-rs closed this Dec 14, 2021

behzadmehmood-rs reopened this Dec 14, 2021

tangxifan approved these changes Dec 14, 2021

View reviewed changes

tangxifan merged commit 6aa5209 into verilog-to-routing:master Dec 14, 2021

tangxifan deleted the api_num_configurable_edges branch December 14, 2021 22:08

tangxifan reviewed Dec 16, 2021

View reviewed changes

vpr/src/device/rr_graph_view.h Show resolved Hide resolved

RRGraphView edges(), num_edges(), configurable_edges(), non_configurable_edges(), num_configurable_edges(), num_non_configurable_edges() Implementation #1917

RRGraphView edges(), num_edges(), configurable_edges(), non_configurable_edges(), num_configurable_edges(), num_non_configurable_edges() Implementation #1917

Uh oh!

Conversation

behzadmehmood-rs commented Nov 18, 2021 • 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!

Uh oh!

Uh oh!

tangxifan Nov 19, 2021

Choose a reason for hiding this comment

Uh oh!

tangxifan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

behzadmehmood-rs commented Dec 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tangxifan commented Dec 10, 2021

Uh oh!

behzadmehmood-rs commented Dec 10, 2021

Uh oh!

behzadmehmood-rs commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tangxifan left a comment • 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!

behzadmehmood-rs commented Dec 13, 2021

Uh oh!

behzadmehmood-rs commented Dec 14, 2021

Uh oh!

behzadmehmood-rs commented Dec 14, 2021

Uh oh!

tangxifan commented Dec 14, 2021

Uh oh!

behzadmehmood-rs commented Dec 14, 2021

Uh oh!

tangxifan commented Dec 14, 2021

Uh oh!

behzadmehmood-rs commented Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tangxifan commented Dec 14, 2021

Uh oh!

behzadmehmood-rs commented Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tangxifan commented Dec 14, 2021

Uh oh!

behzadmehmood-rs commented Dec 14, 2021

Uh oh!

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

Uh oh!

Uh oh!

behzadmehmood-rs commented Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tangxifan commented Dec 14, 2021

Uh oh!

tangxifan commented Dec 14, 2021

Uh oh!

Uh oh!

Uh oh!

behzadmehmood-rs commented Nov 18, 2021 •

edited

Loading

behzadmehmood-rs commented Dec 10, 2021 •

edited

Loading

behzadmehmood-rs commented Dec 13, 2021 •

edited

Loading

tangxifan left a comment •

edited

Loading

behzadmehmood-rs commented Dec 14, 2021 •

edited

Loading

behzadmehmood-rs commented Dec 14, 2021 •

edited

Loading

tangxifan commented Dec 14, 2021 •

edited

Loading

behzadmehmood-rs commented Dec 14, 2021 •

edited

Loading