-
Notifications
You must be signed in to change notification settings - Fork 423
[WIP] Router Lookahead Profiler #2683
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
739029b
aa5570b
c88364a
d406c57
c86ff7c
ddf34eb
0826ad7
e8ea974
0b5a752
cbe1334
80086ff
c747096
e753094
d9f4ee3
dfab0a8
c9801b7
7fe0b55
c8b196e
ed4460d
ab2c731
2db5325
17ab565
595af59
2f10ec5
9bcad70
c05e498
643811b
1c9bb58
deddc8a
971eebc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,23 +3,14 @@ | |
// | ||
|
||
#include <sstream> | ||
#include "lookahead_profiler.h" | ||
#include "vtr_error.h" | ||
#include "vtr_log.h" | ||
|
||
#include "globals.h" | ||
#include "lookahead_profiler.h" | ||
#include "re_cluster_util.h" | ||
#include "router_lookahead_map_utils.h" | ||
#include "vpr_utils.h" | ||
#include "re_cluster_util.h" | ||
|
||
LookaheadProfiler::LookaheadProfiler() | ||
: is_empty(true) { | ||
lookahead_verifier_csv.open("lookahead_verifier_info.csv", std::ios::out); | ||
|
||
if (!lookahead_verifier_csv.is_open()) { | ||
VTR_LOG_WARN("Could not open lookahead_verifier_info.csv"); | ||
return; | ||
} | ||
} | ||
#include "vtr_error.h" | ||
#include "vtr_log.h" | ||
|
||
void LookaheadProfiler::record(int iteration, | ||
int target_net_pin_index, | ||
|
@@ -32,10 +23,14 @@ void LookaheadProfiler::record(int iteration, | |
const auto& rr_graph = device_ctx.rr_graph; | ||
auto& route_ctx = g_vpr_ctx.routing(); | ||
|
||
if (!lookahead_verifier_csv.is_open()) | ||
return; | ||
|
||
// If csv file hasn't been opened, open it and write out column headers | ||
if (is_empty) { | ||
lookahead_verifier_csv.open("lookahead_verifier_info.csv", std::ios::out); | ||
|
||
if (!lookahead_verifier_csv.is_open()) { | ||
VTR_LOG_ERROR("Could not open lookahead_verifier_info.csv", "error"); | ||
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. what is the second argument? |
||
} | ||
|
||
lookahead_verifier_csv | ||
<< "iteration no." | ||
<< ",source node" | ||
|
@@ -63,72 +58,96 @@ void LookaheadProfiler::record(int iteration, | |
is_empty = false; | ||
} | ||
|
||
if (!lookahead_verifier_csv.is_open()) | ||
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. a warning message can be helpful |
||
return; | ||
|
||
// The default value in RouteTree::update_from_heap() is -1; only calls which descend from route_net() | ||
// pass in an iteration value, which is the only context in which we want to profile. | ||
if (iteration < 1) | ||
return; | ||
|
||
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. This function should have a comment giving an overview of how it works, unless that's already covered in the (to be added) lookahead_profiler.cpp file-scope comment that gives the high-level context. |
||
for (size_t i = 2; i < branch_inodes.size(); ++i) { /* Distance one node away is always 0.0 (IPIN->SINK) */ | ||
RRNodeId source_inode = branch_inodes.back(); | ||
RRNodeId sink_inode = branch_inodes.front(); | ||
RRNodeId curr_inode = branch_inodes[i]; | ||
RRNodeId source_inode = branch_inodes.back(); | ||
RRNodeId sink_inode = branch_inodes.front(); | ||
|
||
float total_backward_cost = route_ctx.rr_node_route_inf[sink_inode].backward_path_cost; | ||
float total_backward_delay = route_ctx.rr_node_route_inf[sink_inode].backward_path_delay; | ||
float total_backward_congestion = route_ctx.rr_node_route_inf[sink_inode].backward_path_congestion; | ||
VTR_ASSERT(rr_graph.node_type(source_inode) == SOURCE); | ||
VTR_ASSERT(rr_graph.node_type(sink_inode) == SINK); | ||
|
||
auto current_node = route_ctx.rr_node_route_inf[curr_inode]; | ||
float current_backward_cost = current_node.backward_path_cost; | ||
float current_backward_delay = current_node.backward_path_delay; | ||
float current_backward_congestion = current_node.backward_path_congestion; | ||
/* Get sink node attributes (atom block name, atom block model, cluster type, tile dimensions) */ | ||
|
||
auto [from_x, from_y] = util::get_adjusted_rr_position(curr_inode); | ||
auto [to_x, to_y] = util::get_adjusted_rr_position(sink_inode); | ||
if (atom_block_names.find(sink_inode) == atom_block_names.end()) { | ||
if (net_id != ParentNetId::INVALID() && target_net_pin_index != OPEN) { | ||
atom_block_names[sink_inode] = net_list.block_name(net_list.net_pin_block(net_id, target_net_pin_index)); | ||
|
||
int delta_x = to_x - from_x; | ||
int delta_y = to_y - from_y; | ||
AtomBlockId atom_block_id = g_vpr_ctx.atom().nlist.find_block(atom_block_names[sink_inode]); | ||
atom_block_models[sink_inode] = g_vpr_ctx.atom().nlist.block_model(atom_block_id)->name; | ||
|
||
float djikstra_cost = total_backward_cost - current_backward_cost; | ||
float djikstra_delay = total_backward_delay - current_backward_delay; | ||
float djikstra_congestion = total_backward_congestion - current_backward_congestion; | ||
ClusterBlockId cluster_block_id = atom_to_cluster(atom_block_id); | ||
|
||
float lookahead_cost = router_lookahead.get_expected_cost(curr_inode, sink_inode, cost_params, 0.0); | ||
auto [lookahead_delay, lookahead_congestion] = router_lookahead.get_expected_delay_and_cong(curr_inode, sink_inode, cost_params, 0.0); | ||
cluster_block_types[sink_inode] = g_vpr_ctx.clustering().clb_nlist.block_type(cluster_block_id)->name; | ||
|
||
auto tile_type = physical_tile_type(cluster_block_id); | ||
tile_dimensions[sink_inode] = std::pair(std::to_string(tile_type->width), std::to_string(tile_type->height)); | ||
} else { | ||
atom_block_names[sink_inode] = "--"; | ||
atom_block_models[sink_inode] = "--"; | ||
cluster_block_types[sink_inode] = "--"; | ||
tile_dimensions[sink_inode] = {"--", "--"}; | ||
} | ||
} | ||
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. can't private member variables initialization be done in the constructor? 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. Do you mean that I should initialize them as empty maps in the constructor? 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. What I had in mind was initializing them with all sinks ids as key and proper values. However, this can't be done when LookaheadProfiler has a global instance. 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 think storing multiple std::string values for each sink id is not memory efficient. For std::string_view, vtr::interned_string can be used to avoid unnecessary string copies. 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 decided to change these variables to store IDs and type pointers. Dereferencing the pointers may be less runtime-efficient, but to use |
||
|
||
if (atom_block_names.find(sink_inode) == atom_block_names.end()) { | ||
if (net_id != ParentNetId::INVALID() && target_net_pin_index != OPEN) { | ||
atom_block_names[sink_inode] = net_list.block_name(net_list.net_pin_block(net_id, target_net_pin_index)); | ||
VTR_ASSERT_SAFE(atom_block_models.find(sink_inode) != atom_block_models.end()); | ||
VTR_ASSERT_SAFE(cluster_block_types.find(sink_inode) != cluster_block_types.end()); | ||
VTR_ASSERT_SAFE(tile_dimensions.find(sink_inode) != tile_dimensions.end()); | ||
|
||
AtomBlockId atom_block_id = g_vpr_ctx.atom().nlist.find_block(atom_block_names[sink_inode]); | ||
atom_block_models[sink_inode] = g_vpr_ctx.atom().nlist.block_model(atom_block_id)->name; | ||
std::string block_name = atom_block_names[sink_inode]; | ||
std::string atom_block_model = atom_block_models[sink_inode]; | ||
std::string cluster_block_type = cluster_block_types[sink_inode]; | ||
auto [tile_width, tile_height] = tile_dimensions[sink_inode]; | ||
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. const std::string& don't copy these strings if they are not going to be modified |
||
|
||
ClusterBlockId cluster_block_id = atom_to_cluster(atom_block_id); | ||
/* Iterate through the given path and record information for each node */ | ||
for (size_t i = 2; i < branch_inodes.size(); ++i) { // Distance one node away is always 0. (IPIN->SINK) | ||
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. a more meaningful variable for the iteration number variable |
||
RRNodeId curr_inode = branch_inodes[i]; | ||
|
||
cluster_block_types[sink_inode] = g_vpr_ctx.clustering().clb_nlist.block_type(cluster_block_id)->name; | ||
// Get backwards path cost, delay, and congestion from sink node | ||
t_rr_node_route_inf sink_node_info = route_ctx.rr_node_route_inf[sink_inode]; | ||
float total_backward_cost = sink_node_info.backward_path_cost; | ||
float total_backward_delay = sink_node_info.backward_path_delay; | ||
float total_backward_congestion = sink_node_info.backward_path_congestion; | ||
|
||
// Get backwards path cost, delay, and congestion from current node | ||
t_rr_node_route_inf curr_node_info = route_ctx.rr_node_route_inf[curr_inode]; | ||
float current_backward_cost = curr_node_info.backward_path_cost; | ||
float current_backward_delay = curr_node_info.backward_path_delay; | ||
float current_backward_congestion = curr_node_info.backward_path_congestion; | ||
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 don't undersatnd why these float variabels are needed. I think removing these six variables and directly accessing curr_node_info and sink_node_info makes the coder shorter and readable |
||
|
||
// Get the difference in the coordinates in the current and sink nodes. | ||
// Note: we are not using util::get_xy_deltas() because this always gives positive values | ||
// unless the current node is the source node. Using util::get_adjusted_rr_position therefore | ||
// gives us more information. | ||
auto [from_x, from_y] = util::get_adjusted_rr_position(curr_inode); | ||
auto [to_x, to_y] = util::get_adjusted_rr_position(sink_inode); | ||
|
||
auto tile_type = physical_tile_type(cluster_block_id); | ||
tile_dimensions[sink_inode] = std::pair(std::to_string(tile_type->width), std::to_string(tile_type->height)); | ||
} else { | ||
atom_block_names[sink_inode] = "--"; | ||
atom_block_models[sink_inode] = "--"; | ||
cluster_block_types[sink_inode] = "--"; | ||
tile_dimensions[sink_inode] = {"--", "--"}; | ||
} | ||
} | ||
int delta_x = to_x - from_x; | ||
int delta_y = to_y - from_y; | ||
|
||
VTR_ASSERT_SAFE(atom_block_names.find(sink_inode) != atom_block_names.end()); | ||
VTR_ASSERT_SAFE(atom_block_models.find(sink_inode) != atom_block_models.end()); | ||
VTR_ASSERT_SAFE(cluster_block_types.find(sink_inode) != cluster_block_types.end()); | ||
VTR_ASSERT_SAFE(tile_dimensions.find(sink_inode) != tile_dimensions.end()); | ||
// Calculate the actual cost, delay, and congestion from the current node to the sink. | ||
float actual_cost = total_backward_cost - current_backward_cost; | ||
float actual_delay = total_backward_delay - current_backward_delay; | ||
float actual_congestion = total_backward_congestion - current_backward_congestion; | ||
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. It would be more readable if these three lines appear immediately after getting the current and sink node info |
||
|
||
std::string block_name = atom_block_names[sink_inode]; | ||
std::string atom_block_model = atom_block_models[sink_inode]; | ||
std::string cluster_block_type = cluster_block_types[sink_inode]; | ||
auto [tile_width, tile_height] = tile_dimensions[sink_inode]; | ||
// Get the cost, delay, and congestion estimates made by the lookahead. | ||
// Note: lookahead_cost = lookahead_delay * criticality + lookahead_congestion * (1. - criticality) | ||
float lookahead_cost = router_lookahead.get_expected_cost(curr_inode, sink_inode, cost_params, 0.0); | ||
auto [lookahead_delay, lookahead_congestion] = router_lookahead.get_expected_delay_and_cong(curr_inode, sink_inode, cost_params, 0.0); | ||
|
||
// Get the current node's type and length | ||
std::string node_type_str = rr_graph.node_type_string(curr_inode); | ||
std::string node_length = (node_type_str == "CHANX" || node_type_str == "CHANX") | ||
? std::to_string(rr_graph.node_length(curr_inode)) | ||
: "--"; | ||
|
||
/* Write out all info */ | ||
|
||
lookahead_verifier_csv << iteration << ","; // iteration no. | ||
lookahead_verifier_csv << source_inode << ","; // source node | ||
lookahead_verifier_csv << sink_inode << ","; // sink node | ||
|
@@ -143,9 +162,9 @@ void LookaheadProfiler::record(int iteration, | |
lookahead_verifier_csv << i << ","; // num. nodes from sink | ||
lookahead_verifier_csv << delta_x << ","; // delta x | ||
lookahead_verifier_csv << delta_y << ","; // delta y | ||
lookahead_verifier_csv << djikstra_cost << ","; // actual cost | ||
lookahead_verifier_csv << djikstra_delay << ","; // actual delay | ||
lookahead_verifier_csv << djikstra_congestion << ","; // actual congestion | ||
lookahead_verifier_csv << actual_cost << ","; // actual cost | ||
lookahead_verifier_csv << actual_delay << ","; // actual delay | ||
lookahead_verifier_csv << actual_congestion << ","; // actual congestion | ||
lookahead_verifier_csv << lookahead_cost << ","; // predicted cost | ||
lookahead_verifier_csv << lookahead_delay << ","; // predicted delay | ||
lookahead_verifier_csv << lookahead_congestion << ","; // predicted congestion | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,35 @@ | |
|
||
#include <fstream> | ||
#include <thread> | ||
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. why is thread needed? |
||
#include "rr_graph_fwd.h" | ||
|
||
#include "connection_router_interface.h" | ||
#include "router_lookahead.h" | ||
#include "rr_graph_fwd.h" | ||
|
||
/** | ||
* @brief A class which records information used to profile the router lookahead: most importantly, | ||
* the actual cost (delay and congestion) from nodes to the sink to which they have been routed, as | ||
* well as the lookahead's estimation of this cost. | ||
*/ | ||
class LookaheadProfiler { | ||
public: | ||
LookaheadProfiler(); | ||
LookaheadProfiler() | ||
: is_empty(true) {} | ||
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. delete default constuctors and operators, assuming this class is not intented to be copied or moved |
||
|
||
/** | ||
* @brief Record information on nodes on a path from a source to a sink. | ||
* | ||
* @param iteration The router iteration. | ||
* @param target_net_pin_index Target pin of this sink in the net. | ||
* @param cost_params | ||
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. Suggest briefly commenting each input's purpose. 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 need a high-level comment of how the lookahead profiler works somewhere (file level, or maybe on this routine). |
||
* @param router_lookahead | ||
* @param net_id | ||
* @param net_list | ||
* @param branch_inodes A path from a sink to its source, as a vector of nodes. | ||
* | ||
* @warning | ||
* branch_inodes must be a backwards path, from a sink node to a source node. | ||
*/ | ||
void record(int iteration, | ||
int target_net_pin_index, | ||
const t_conn_cost_params& cost_params, | ||
|
@@ -20,11 +41,17 @@ class LookaheadProfiler { | |
std::vector<RRNodeId> branch_inodes); | ||
|
||
private: | ||
///@breif The output filestream. | ||
std::ofstream lookahead_verifier_csv; | ||
///@brief Whether the output file is empty/not yet opened. | ||
bool is_empty; | ||
///@brief A map from sink node IDs to the names of their atom blocks. | ||
std::unordered_map<RRNodeId, std::string> atom_block_names; | ||
///@brief A map from sink node IDs to the names of the models of their atom blocks. | ||
std::unordered_map<RRNodeId, std::string> atom_block_models; | ||
///@brief A map from sink node IDs to the names of the types of their clusters. | ||
std::unordered_map<RRNodeId, std::string> cluster_block_types; | ||
///@brief A map from sink node IDs to the dimensions of their tiles (width, height). | ||
std::unordered_map<RRNodeId, std::pair<std::string, std::string>> tile_dimensions; | ||
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. a good practice for nameing private members is adding an underline to the end of names |
||
}; | ||
|
||
|
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.
this if statement is true only once? is that right?
In this case, I think it is better to open the file and write the first line in the constructor.
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.
The only issue is that the profiler object is initialized in
RoutingContext
, so if opening the file is done in the constructor, it will happen even if the option to use it isn't onThere 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.
In another comment, I mentioned that we can also pass the flag indicating wether the profiler is on to the constructor. With this flag passed to the constructor, you can decide to open the file or not.
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.
I'm not sure how to do this without creating a constructor for RoutingContext and VprContext as well; would this be okay?
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.
You're right. RoutingContext has a global instance, meaning that LookaheadProfiler's constructor is called before parsing command line options.
I think we shouldn avoid adding anything to the global context unless it is really necessary. Instead, we could ceate a LookaheadProfiler instance at the start of the routing stage and pass it through a function call hierarchy down to its call site.
Uh oh!
There was an error while loading. Please reload this page.
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.
If you decide to keep LookaheadProfiler instance global, you should free the memory allocated for its member variables after it is no longer needed. This is because the global instance doesn't go out of scope