-
Notifications
You must be signed in to change notification settings - Fork 415
Remove legacy data structure rr_node_indices
from DeviceContext
#1828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -1,8 +1,7 @@ | ||||
#include "vtr_assert.h" | ||||
#include "rr_spatial_lookup.h" | ||||
|
||||
RRSpatialLookup::RRSpatialLookup(t_rr_node_indices& rr_node_indices) | ||||
: rr_node_indices_(rr_node_indices) { | ||||
RRSpatialLookup::RRSpatialLookup() { | ||||
} | ||||
|
||||
RRNodeId RRSpatialLookup::find_node(int x, | ||||
|
@@ -273,3 +272,26 @@ void RRSpatialLookup::resize_nodes(int x, | |||
std::max(rr_node_indices_[type].dim_size(2), size_t(side) + 1)}); | ||||
} | ||||
} | ||||
|
||||
void RRSpatialLookup::reorder(const vtr::vector<RRNodeId, RRNodeId> dest_order) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd expect some sort of unit-test for this method ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right. This feature is already under testing in one of the test case in vtr-verilog-to-routing/vtr_flow/tasks/regression_tests/vtr_reg_basic/basic_timing/config/config.txt Line 32 in 29a9fd5
I am not sure if this can be categorized as a unit-test. If not, we should think about it once we can extract the |
||||
// update rr_node_indices, a map to optimize rr_index lookups | ||||
for (auto& grid : rr_node_indices_) { | ||||
for (size_t x = 0; x < grid.dim_size(0); x++) { | ||||
for (size_t y = 0; y < grid.dim_size(1); y++) { | ||||
for (size_t s = 0; s < grid.dim_size(2); s++) { | ||||
for (auto& node : grid[x][y][s]) { | ||||
if (node != OPEN) { | ||||
node = size_t(dest_order[RRNodeId(node)]); | ||||
} | ||||
} | ||||
} | ||||
} | ||||
} | ||||
} | ||||
} | ||||
|
||||
void RRSpatialLookup::clear() { | ||||
for (auto& data : rr_node_indices_) { | ||||
data.clear(); | ||||
} | ||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
#define RR_SPATIAL_LOOKUP_H | ||
|
||
#include "vtr_geometry.h" | ||
#include "vtr_vector.h" | ||
#include "vpr_types.h" | ||
|
||
/******************************************************************** | ||
|
@@ -17,7 +18,7 @@ class RRSpatialLookup { | |
/* -- Constructors -- */ | ||
public: | ||
/* Explicitly define the only way to create an object */ | ||
explicit RRSpatialLookup(t_rr_node_indices& rr_node_indices); | ||
explicit RRSpatialLookup(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we don't take a parameter anymore, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sense to me. Will remove it. |
||
|
||
/* Disable copy constructors and copy assignment operator | ||
* This is to avoid accidental copy because it could be an expensive operation considering that the | ||
|
@@ -185,6 +186,12 @@ class RRSpatialLookup { | |
t_rr_type type, | ||
e_side side); | ||
|
||
/* Reorder the internal look up to be more memory efficient */ | ||
void reorder(const vtr::vector<RRNodeId, RRNodeId> dest_order); | ||
|
||
/* Clear all the data inside */ | ||
void clear(); | ||
|
||
/* -- Internal data queries -- */ | ||
private: | ||
/* An internal API to find all the nodes in a specific location with a given type | ||
|
@@ -199,17 +206,8 @@ class RRSpatialLookup { | |
|
||
/* -- Internal data storage -- */ | ||
private: | ||
/* TODO: When the refactoring effort finishes, | ||
* the data structure will be the owner of the data storages. | ||
* That is why the reference is used here. | ||
* It can avoid a lot of code changes once the refactoring is finished | ||
* (there is no function get data directly through the rr_node_indices in DeviceContext). | ||
* If pointers are used, it may cause many codes in client functions | ||
* or inside the data structures to be changed later. | ||
* That explains why the reference is used here temporarily | ||
*/ | ||
/* Fast look-up: TODO: Should rework the data type. Currently it is based on a 3-dimensional arrqay mater where some dimensions must always be accessed with a specific index. Such limitation should be overcome */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo to fix: 3-dimensional array matrix |
||
t_rr_node_indices& rr_node_indices_; | ||
t_rr_node_indices rr_node_indices_; | ||
}; | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we have the RRGraphView and RRGraphBuilder APIs, we should make doxygen documentation for them. Best done as a follow-up issue, so I'll file an issue for that (make doxygen-compatible comments, and add to the VPR API on the read-the-docs page).