Skip to content

Clean-up DeviceContext (Remove shadowed data structure) #1962

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

Merged
merged 6 commits into from
Feb 9, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion utils/route_diag/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ static void do_one_route(int source_node, int sink_node,
ConnectionRouter<BinaryHeap> router(
device_ctx.grid,
*router_lookahead,
device_ctx.rr_nodes,
device_ctx.rr_graph.rr_nodes(),
&device_ctx.rr_graph,
device_ctx.rr_rc_data,
device_ctx.rr_graph.rr_switch(),
Expand Down
14 changes: 6 additions & 8 deletions vpr/src/base/read_route.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ bool read_route(const char* route_file, const t_router_opts& router_opts, bool v
recompute_occupancy_from_scratch();

/* Note: This pres_fac is not necessarily correct since it isn't the first routing iteration*/
OveruseInfo overuse_info(device_ctx.rr_nodes.size());
OveruseInfo overuse_info(device_ctx.rr_graph.num_nodes());
pathfinder_update_acc_cost_and_overuse_info(router_opts.acc_fac, overuse_info);

reserve_locally_used_opins(&small_heap, router_opts.initial_pres_fac,
Expand Down Expand Up @@ -505,21 +505,19 @@ static bool check_rr_graph_connectivity(RRNodeId prev_node, RRNodeId node) {
if (prev_node == node) return false;

auto& device_ctx = g_vpr_ctx.device();
const auto& rr_graph = device_ctx.rr_nodes;
/*TODO We need to remove temp_rr_graph once rr_graph is resolved*/
const auto& temp_rr_graph = device_ctx.rr_graph;
const auto& rr_graph = device_ctx.rr_graph;
// If it's starting a new sub branch this is ok
if (device_ctx.rr_graph.node_type(prev_node) == SINK) return true;
if (rr_graph.node_type(prev_node) == SINK) return true;

for (RREdgeId edge : rr_graph.edge_range(prev_node)) {
//If the sink node is reachable by previous node return true
if (rr_graph.edge_sink_node(edge) == node) {
if (rr_graph.rr_nodes().edge_sink_node(edge) == node) {
return true;
}

// If there are any non-configurable branches return true
short edge_switch = rr_graph.edge_switch(edge);
if (!(temp_rr_graph.rr_switch_inf(RRSwitchId(edge_switch)).configurable())) return true;
short edge_switch = rr_graph.rr_nodes().edge_switch(edge);
if (!(rr_graph.rr_switch_inf(RRSwitchId(edge_switch)).configurable())) return true;
}

// If it's part of a non configurable node list, return true
Expand Down
6 changes: 3 additions & 3 deletions vpr/src/base/vpr_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ struct DeviceContext : public Context {
/*
* Structures to define the routing architecture of the FPGA.
*/
t_rr_graph_storage rr_nodes; // autogenerated in build_rr_graph
//t_rr_graph_storage rr_nodes; // autogenerated in build_rr_graph
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this line


vtr::vector<RRIndexedDataId, t_rr_indexed_data> rr_indexed_data; // [0 .. num_rr_indexed_data-1]

Expand All @@ -163,12 +163,12 @@ struct DeviceContext : public Context {
/* A writeable view of routing resource graph to be the ONLY database
* for routing resource graph builder functions.
*/
RRGraphBuilder rr_graph_builder{&rr_nodes, &rr_node_metadata, &rr_edge_metadata};
RRGraphBuilder rr_graph_builder{&rr_node_metadata, &rr_edge_metadata};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also delete the line 150 in this file


/* A read-only view of routing resource graph to be the ONLY database
* for client functions: GUI, placer, router, timing analyzer etc.
*/
RRGraphView rr_graph{rr_nodes, rr_graph_builder.node_lookup(), rr_indexed_data, rr_graph_builder.rr_segments(), rr_graph_builder.rr_switch()};
RRGraphView rr_graph{rr_graph_builder.rr_nodes(), rr_graph_builder.node_lookup(), rr_indexed_data, rr_graph_builder.rr_segments(), rr_graph_builder.rr_switch()};
int num_arch_switches;
t_arch_switch_inf* arch_switch_inf; // [0..(num_arch_switches-1)]

Expand Down
12 changes: 6 additions & 6 deletions vpr/src/device/rr_graph_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@

//#include "globals.h"

RRGraphBuilder::RRGraphBuilder(t_rr_graph_storage* node_storage,
MetadataStorage<int>* rr_node_metadata,
MetadataStorage<std::tuple<int, int, short>>* rr_edge_metadata)
: node_storage_(*node_storage)
, rr_node_metadata_(*rr_node_metadata)
RRGraphBuilder::RRGraphBuilder(
MetadataStorage<int>* rr_node_metadata,
MetadataStorage<std::tuple<int, int, short>>* rr_edge_metadata)
: rr_node_metadata_(*rr_node_metadata)
, rr_edge_metadata_(*rr_edge_metadata) {
}

t_rr_graph_storage& RRGraphBuilder::node_storage() {
t_rr_graph_storage& RRGraphBuilder::rr_nodes() {
return node_storage_;
}

Expand Down Expand Up @@ -59,6 +58,7 @@ void RRGraphBuilder::add_node_to_all_locs(RRNodeId node) {

void RRGraphBuilder::clear() {
node_lookup_.clear();
node_storage_.clear();
rr_segments_.clear();
rr_switch_inf_.clear();
}
Expand Down
15 changes: 10 additions & 5 deletions vpr/src/device/rr_graph_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ class RRGraphBuilder {
/* -- Constructors -- */
public:
/* See detailed comments about the data structures in the internal data storage section of this file */
RRGraphBuilder(t_rr_graph_storage* node_storage,
MetadataStorage<int>* rr_node_metadata,
MetadataStorage<std::tuple<int, int, short>>* rr_edge_metadata);
RRGraphBuilder(
MetadataStorage<int>* rr_node_metadata,
MetadataStorage<std::tuple<int, int, short>>* rr_edge_metadata);

/* Disable copy constructors and copy assignment operator
* This is to avoid accidental copy because it could be an expensive operation considering that the
Expand All @@ -37,7 +37,7 @@ class RRGraphBuilder {
/* -- Mutators -- */
public:
/** @brief Return a writable object for rr_nodes */
t_rr_graph_storage& node_storage();
t_rr_graph_storage& rr_nodes();
/** @brief Return a writable object for update the fast look-up of rr_node */
RRSpatialLookup& node_lookup();

Expand Down Expand Up @@ -182,6 +182,11 @@ class RRGraphBuilder {
inline void emplace_back_edge(RRNodeId src, RRNodeId dest, short edge_switch) {
node_storage_.emplace_back_edge(src, dest, edge_switch);
}
/** @brief Append 1 more RR node to the RR graph. */
inline void emplace_back() {
// No edges can be assigned if mutating the rr node array.
node_storage_.emplace_back();
}

/** @brief alloc_and_load_edges; It adds a batch of edges. */
inline void alloc_and_load_edges(const t_rr_edge_info_set* rr_edges_to_create) {
Expand Down Expand Up @@ -286,7 +291,7 @@ class RRGraphBuilder {
* That explains why the reference is used here temporarily
*/
/* node-level storage including edge storages */
t_rr_graph_storage& node_storage_;
t_rr_graph_storage node_storage_;
/* Fast look-up for rr nodes */
RRSpatialLookup node_lookup_;

Expand Down
18 changes: 16 additions & 2 deletions vpr/src/device/rr_graph_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,23 @@ class RRGraphView {
* }
*/
inline vtr::StrongIdRange<RRNodeId> nodes() const {
return vtr::StrongIdRange<RRNodeId>(RRNodeId(0), RRNodeId(size()));
return vtr::StrongIdRange<RRNodeId>(RRNodeId(0), RRNodeId(num_nodes()));
}

/** @brief Return number of nodes. This function is inlined for runtime optimization. */
inline size_t size() const {
inline size_t num_nodes() const {
return node_storage_.size();
}
/** @brief Is the RR graph currently empty? */
inline bool empty() const {
return node_storage_.empty();
}

/** @brief Returns a range of RREdgeId's belonging to RRNodeId id.
* If this range is empty, then RRNodeId id has no edges.*/
inline vtr::StrongIdRange<RREdgeId> edge_range(RRNodeId id) const {
return vtr::StrongIdRange<RREdgeId>(node_storage_.first_edge(id), node_storage_.last_edge(id));
}

/** @brief Get the type of a routing resource node. This function is inlined for runtime optimization. */
inline t_rr_type node_type(RRNodeId node) const {
Expand Down Expand Up @@ -419,6 +429,10 @@ class RRGraphView {
const RRSpatialLookup& node_lookup() const {
return node_lookup_;
}
/** @brief Return the node-level storage structure for queries from client functions */
const t_rr_graph_storage& rr_nodes() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware of the runtime overhead. Can you try to make this API inline?

return node_storage_;
}

/* -- Internal data storage -- */
/* Note: only read-only object or data structures are allowed!!! */
Expand Down
30 changes: 15 additions & 15 deletions vpr/src/draw/draw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ void alloc_draw_structs(const t_arch* arch) {
/* Space is allocated for draw_rr_node but not initialized because we do *
* not yet know information about the routing resources. */
draw_state->draw_rr_node = (t_draw_rr_node*)vtr::malloc(
device_ctx.rr_nodes.size() * sizeof(t_draw_rr_node));
device_ctx.rr_graph.num_nodes() * sizeof(t_draw_rr_node));

draw_state->arch_info = arch;

Expand Down Expand Up @@ -980,10 +980,10 @@ void init_draw_coords(float width_val) {
return; //do not initialize only if --disp off and --save_graphics off
/* Each time routing is on screen, need to reallocate the color of each *
* rr_node, as the number of rr_nodes may change. */
if (device_ctx.rr_nodes.size() != 0) {
if (rr_graph.num_nodes() != 0) {
draw_state->draw_rr_node = (t_draw_rr_node*)vtr::realloc(
draw_state->draw_rr_node,
(device_ctx.rr_nodes.size()) * sizeof(t_draw_rr_node));
(rr_graph.num_nodes()) * sizeof(t_draw_rr_node));
/*FIXME: the type cast should be eliminated by making draw_rr_node adapt RRNodeId */
for (const RRNodeId& rr_id : rr_graph.nodes()) {
draw_state->draw_rr_node[(size_t)rr_id].color = DEFAULT_RR_NODE_COLOR;
Expand Down Expand Up @@ -1310,7 +1310,7 @@ static void draw_routing_costs(ezgl::renderer* g) {

float min_cost = std::numeric_limits<float>::infinity();
float max_cost = -min_cost;
std::vector<float> rr_node_costs(device_ctx.rr_nodes.size(), 0.);
std::vector<float> rr_node_costs(device_ctx.rr_graph.num_nodes(), 0.);

for (const RRNodeId& rr_id : device_ctx.rr_graph.nodes()) {
float cost = 0.;
Expand Down Expand Up @@ -1694,7 +1694,7 @@ static void draw_rr_edges(int inode, ezgl::renderer* g) {
to_node = size_t(rr_graph.edge_sink_node(rr_node, iedge));
to_type = rr_graph.node_type(RRNodeId(to_node));
to_ptc_num = rr_graph.node_ptc_num(RRNodeId(to_node));
bool edge_configurable = device_ctx.rr_nodes[inode].edge_is_configurable(iedge);
bool edge_configurable = rr_graph.rr_nodes()[inode].edge_is_configurable(iedge);

switch (from_type) {
case OPIN:
Expand Down Expand Up @@ -2288,7 +2288,7 @@ static void draw_rr_pin(int inode, const ezgl::color& color, ezgl::renderer* g)
* the physical pin is on. */
void draw_get_rr_pin_coords(int inode, float* xcen, float* ycen, const e_side& pin_side) {
auto& device_ctx = g_vpr_ctx.device();
draw_get_rr_pin_coords(device_ctx.rr_nodes[inode], xcen, ycen, pin_side);
draw_get_rr_pin_coords(device_ctx.rr_graph.rr_nodes()[inode], xcen, ycen, pin_side);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tangxifan
Can you Please Have a look on this line ?

Copy link
Contributor

@tangxifan tangxifan Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@umariqbal-rs This line looks o.k. We will refactor the functions in the draw.cpp later. Ideally, we want these function to use rr_graph and RRNodeId, rather than sending a rr_node data structure.

}

void draw_get_rr_pin_coords(const t_rr_node& node, float* xcen, float* ycen, const e_side& pin_side) {
Expand Down Expand Up @@ -2357,7 +2357,7 @@ static void draw_rr_src_sink(int inode, ezgl::color color, ezgl::renderer* g) {
const auto& rr_graph = device_ctx.rr_graph;

float xcen, ycen;
draw_get_rr_src_sink_coords(device_ctx.rr_nodes[inode], &xcen, &ycen);
draw_get_rr_src_sink_coords(rr_graph.rr_nodes()[inode], &xcen, &ycen);

g->set_color(color);

Expand Down Expand Up @@ -2786,7 +2786,7 @@ static int draw_check_rr_node_hit(float click_x, float click_y) {
case SOURCE:
case SINK: {
float xcen, ycen;
draw_get_rr_src_sink_coords(device_ctx.rr_nodes[inode], &xcen, &ycen);
draw_get_rr_src_sink_coords(rr_graph.rr_nodes()[inode], &xcen, &ycen);

// Now check if we clicked on this pin
if (click_x >= xcen - draw_coords->pin_size && click_x <= xcen + draw_coords->pin_size && click_y >= ycen - draw_coords->pin_size && click_y <= ycen + draw_coords->pin_size) {
Expand Down Expand Up @@ -2829,7 +2829,7 @@ void draw_expand_non_configurable_rr_nodes_recurr(int from_node,

for (t_edge_size iedge = 0;
iedge < rr_graph.num_edges(RRNodeId(from_node)); ++iedge) {
bool edge_configurable = device_ctx.rr_nodes[from_node].edge_is_configurable(iedge);
bool edge_configurable = rr_graph.rr_nodes()[from_node].edge_is_configurable(iedge);
int to_node = size_t(rr_graph.edge_sink_node(RRNodeId(from_node), iedge));

if (!edge_configurable && !expanded_nodes.count(to_node)) {
Expand Down Expand Up @@ -3355,7 +3355,7 @@ static void draw_pin_to_sink(int ipin_node, int sink_node, ezgl::renderer* g) {
draw_get_rr_pin_coords(ipin_node, &x1, &y1, pin_side);

float x2 = 0, y2 = 0;
draw_get_rr_src_sink_coords(device_ctx.rr_nodes[sink_node], &x2, &y2);
draw_get_rr_src_sink_coords(rr_graph.rr_nodes()[sink_node], &x2, &y2);

g->draw_line({x1, y1}, {x2, y2});

Expand All @@ -3370,7 +3370,7 @@ static void draw_source_to_pin(int source_node, int opin_node, ezgl::renderer* g
const auto& rr_graph = device_ctx.rr_graph;

float x1 = 0, y1 = 0;
draw_get_rr_src_sink_coords(device_ctx.rr_nodes[source_node], &x1, &y1);
draw_get_rr_src_sink_coords(rr_graph.rr_nodes()[source_node], &x1, &y1);

/* Draw the line for each ipin on different sides */
for (const e_side& pin_side : SIDES) {
Expand Down Expand Up @@ -4141,7 +4141,7 @@ static void draw_router_expansion_costs(ezgl::renderer* g) {
auto& device_ctx = g_vpr_ctx.device();
auto& routing_ctx = g_vpr_ctx.routing();

std::vector<float> rr_costs(device_ctx.rr_nodes.size());
std::vector<float> rr_costs(device_ctx.rr_graph.num_nodes());

for (const RRNodeId& rr_id : device_ctx.rr_graph.nodes()) {
float cost = get_router_expansion_cost(
Expand Down Expand Up @@ -4198,11 +4198,11 @@ static void draw_rr_costs(ezgl::renderer* g, const std::vector<float>& rr_costs,
|| draw_state->show_router_expansion_cost == DRAW_ROUTER_EXPANSION_COST_KNOWN_WITH_EDGES
|| draw_state->show_router_expansion_cost == DRAW_ROUTER_EXPANSION_COST_EXPECTED_WITH_EDGES);

VTR_ASSERT(rr_costs.size() == device_ctx.rr_nodes.size());
VTR_ASSERT(rr_costs.size() == rr_graph.num_nodes());

float min_cost = std::numeric_limits<float>::infinity();
float max_cost = -min_cost;
for (const RRNodeId& rr_id : device_ctx.rr_graph.nodes()) {
for (const RRNodeId& rr_id : rr_graph.nodes()) {
if (std::isnan(rr_costs[(size_t)rr_id])) continue;

min_cost = std::min(min_cost, rr_costs[(size_t)rr_id]);
Expand All @@ -4214,7 +4214,7 @@ static void draw_rr_costs(ezgl::renderer* g, const std::vector<float>& rr_costs,

//Draw the nodes in ascending order of value, this ensures high valued nodes
//are not overdrawn by lower value ones (e.g-> when zoomed-out far)
std::vector<int> nodes(device_ctx.rr_nodes.size());
std::vector<int> nodes(rr_graph.num_nodes());
std::iota(nodes.begin(), nodes.end(), 0);
auto cmp_ascending_cost = [&](int lhs_node, int rhs_node) {
if (lowest_cost_first) {
Expand Down
2 changes: 1 addition & 1 deletion vpr/src/draw/search_bar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void search_and_highlight(GtkWidget* /*widget*/, ezgl::application* app) {
ss >> rr_node_id;

// valid rr node id check
if (rr_node_id < 0 || rr_node_id >= int(device_ctx.rr_nodes.size())) {
if (rr_node_id < 0 || rr_node_id >= int(device_ctx.rr_graph.num_nodes())) {
warning_dialog_box("Invalid RR Node ID");
app->refresh_drawing();
return;
Expand Down
2 changes: 1 addition & 1 deletion vpr/src/pack/post_routing_pb_pin_fixup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ static void update_cluster_pin_with_post_routing_results(const DeviceContext& de
if (!rr_node) {
continue;
}
VTR_ASSERT((size_t)rr_node < device_ctx.rr_nodes.size());
VTR_ASSERT((size_t)rr_node < device_ctx.rr_graph.num_nodes());

/* If the node has been visited on the other side, we just skip it */
if (visited_rr_nodes.end() != std::find(visited_rr_nodes.begin(), visited_rr_nodes.end(), RRNodeId(rr_node))) {
Expand Down
2 changes: 1 addition & 1 deletion vpr/src/power/power.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,7 @@ void power_routing_init(const t_det_routing_arch* routing_arch) {
}

/* Initialize RR Graph Structures */
rr_node_power = (t_rr_node_power*)vtr::calloc(device_ctx.rr_nodes.size(),
rr_node_power = (t_rr_node_power*)vtr::calloc(rr_graph.num_nodes(),
sizeof(t_rr_node_power));
for (const RRNodeId& rr_id : device_ctx.rr_graph.nodes()) {
rr_node_power[(size_t)rr_id].driver_switch_type = OPEN;
Expand Down
4 changes: 2 additions & 2 deletions vpr/src/route/annotate_routing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ vtr::vector<RRNodeId, ClusterNetId> annotate_rr_node_nets(const DeviceContext& d
const auto& rr_graph = device_ctx.rr_graph;

vtr::vector<RRNodeId, ClusterNetId> rr_node_nets;
rr_node_nets.resize(device_ctx.rr_nodes.size(), ClusterNetId::INVALID());
rr_node_nets.resize(rr_graph.num_nodes(), ClusterNetId::INVALID());

for (auto net_id : clustering_ctx.clb_nlist.nets()) {
if (clustering_ctx.clb_nlist.net_is_ignored(net_id)) {
Expand All @@ -52,7 +52,7 @@ vtr::vector<RRNodeId, ClusterNetId> annotate_rr_node_nets(const DeviceContext& d
* whose capacity is 1
*/
if ((rr_node_nets[rr_node])
&& (1 == device_ctx.rr_nodes.node_capacity(rr_node))
&& (1 == rr_graph.node_capacity(rr_node))
&& (net_id != rr_node_nets[rr_node])) {
VPR_FATAL_ERROR(VPR_ERROR_ANALYSIS,
"Detect two nets '%s' and '%s' that are mapped to the same rr_node '%ld'!\n%s\n",
Expand Down
10 changes: 5 additions & 5 deletions vpr/src/route/check_route.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ void check_route(enum e_route_type route_type, e_check_route_option check_route_

check_locally_used_clb_opins(route_ctx.clb_opins_used_locally, route_type);

auto connected_to_route = std::make_unique<bool[]>(device_ctx.rr_nodes.size());
std::fill_n(connected_to_route.get(), device_ctx.rr_nodes.size(), false);
auto connected_to_route = std::make_unique<bool[]>(rr_graph.num_nodes());
std::fill_n(connected_to_route.get(), rr_graph.num_nodes(), false);

max_pins = 0;
for (auto net_id : cluster_ctx.clb_nlist.nets())
Expand Down Expand Up @@ -536,7 +536,7 @@ void recompute_occupancy_from_scratch() {
/* Will always be 0 for pads or SINK classes. */
for (ipin = 0; ipin < num_local_opins; ipin++) {
inode = route_ctx.clb_opins_used_locally[blk_id][iclass][ipin];
VTR_ASSERT(inode >= 0 && inode < (ssize_t)device_ctx.rr_nodes.size());
VTR_ASSERT(inode >= 0 && inode < (ssize_t)device_ctx.rr_graph.num_nodes());
route_ctx.rr_node_route_inf[inode].set_occ(route_ctx.rr_node_route_inf[inode].occ() + 1);
}
}
Expand Down Expand Up @@ -592,9 +592,9 @@ static void check_node_and_range(int inode, enum e_route_type route_type) {

auto& device_ctx = g_vpr_ctx.device();

if (inode < 0 || inode >= (int)device_ctx.rr_nodes.size()) {
if (inode < 0 || inode >= (int)device_ctx.rr_graph.num_nodes()) {
VPR_FATAL_ERROR(VPR_ERROR_ROUTE,
"in check_node_and_range: rr_node #%d is out of legal, range (0 to %d).\n", inode, device_ctx.rr_nodes.size() - 1);
"in check_node_and_range: rr_node #%d is out of legal, range (0 to %d).\n", inode, device_ctx.rr_graph.num_nodes() - 1);
}
check_rr_node(inode, route_type, device_ctx);
}
Expand Down
Loading