From 3620dfd01415fccb10ec6ab9709429a524d6a76e Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 8 May 2023 16:01:07 -0400 Subject: [PATCH 01/29] removed a single comparison function to a if condition (item 1 in issue 2262) --- vpr/src/place/noc_place_utils.cpp | 6 +----- vpr/src/place/noc_place_utils.h | 21 --------------------- vpr/src/place/place.cpp | 4 ++-- 3 files changed, 3 insertions(+), 28 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index ce144c51798..267d9eccc96 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -374,7 +374,7 @@ int check_noc_placement_costs(const t_placer_costs& costs, double error_toleranc } // only check the recomputed cost if it is above our expected latency cost threshold of 1 picosecond, otherwise there is no point in checking it - if (check_recomputed_noc_latency_cost(noc_latency_cost_check)) { + if (noc_latency_cost_check > MIN_EXPECTED_NOC_LATENCY_COST) { // check whether the latency placement cost is within the error tolerance if (fabs(noc_latency_cost_check - costs.noc_latency_cost) > costs.noc_latency_cost * error_tolerance) { VTR_LOG_ERROR( @@ -452,10 +452,6 @@ int get_number_of_traffic_flows_with_latency_cons_met(void) { return count_of_achieved_latency_cons; } -bool check_recomputed_noc_latency_cost(float recomputed_cost) { - return (recomputed_cost < MIN_EXPECTED_NOC_LATENCY_COST) ? false : true; -} - void allocate_and_load_noc_placement_structs(void) { auto& noc_ctx = g_vpr_ctx.noc(); diff --git a/vpr/src/place/noc_place_utils.h b/vpr/src/place/noc_place_utils.h index 70e597d2f16..42500013169 100644 --- a/vpr/src/place/noc_place_utils.h +++ b/vpr/src/place/noc_place_utils.h @@ -375,27 +375,6 @@ double calculate_traffic_flow_latency_cost(const std::vector& traffic */ int get_number_of_traffic_flows_with_latency_cons_met(void); -/** - * @brief This verifies whether we need to compare the recomputed noc latency - * cost to the current noc latency cost. During placement there is a function - * called "recompute_costs_from_scratch" that regularily checks whether the - * NoC latency cost updates after each move match a complete recalculation of - * the NoC latency cost at the current state. This will fail when the latency - * costs get really low. So we expect the latency costs to never actually go - * below one picosecond, so we check whether a recomputed cost is below this - * threshold and if it is then indicate that there is no need to verify the - * NoC latency cost. This is acceptable since the only case where the NoC - * latency cost will be below one picosecond is when the weighting on the - * latency component of the cost is extremely low (ie. 0) and when most if not - * all the latency constraints have been met (so this is also close to 0). There - * is no need to check and compare the cost here. - * - * @param recomputed_cost The newly computed NoC latency cost for a given - * placement state. - * - */ -bool check_recomputed_noc_latency_cost(float recomputed_cost); - /** * @brief There are a number of static datastructures which are local * to 'noc_place_utils.cpp'. THe purpose of these datastructures is diff --git a/vpr/src/place/place.cpp b/vpr/src/place/place.cpp index 9fb50b2fac6..80c642052eb 100644 --- a/vpr/src/place/place.cpp +++ b/vpr/src/place/place.cpp @@ -1247,8 +1247,8 @@ static void recompute_costs_from_scratch(const t_placer_opts& placer_opts, costs->noc_aggregate_bandwidth_cost = new_noc_aggregate_bandwidth_cost; // only check if the recomputed cost and the current noc latency cost are within the error tolerance if the cost is above 1 picosecond. - // Otherwise there is no need to check (we expect the latency cost to be above the threshold of 1 picosecond) - if (check_recomputed_noc_latency_cost(new_noc_latency_cost)) { + // Otherwise, there is no need to check (we expect the latency cost to be above the threshold of 1 picosecond) + if (new_noc_latency_cost > MIN_EXPECTED_NOC_LATENCY_COST) { if (fabs( new_noc_latency_cost - costs->noc_latency_cost) From 83a57b335bc8346b1ca9458b554c5049b441def6 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 8 May 2023 17:58:59 -0400 Subject: [PATCH 02/29] fixed some comments typos --- vpr/src/place/noc_place_utils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 529addde499..ae3c4e17ea1 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -489,7 +489,7 @@ bool check_for_router_swap(int user_supplied_noc_router_swap_percentage) { } e_create_move propose_router_swap(t_pl_blocks_to_be_moved& blocks_affected, float rlim) { - // need to access all the router cluster blocks int he design + // need to access all the router cluster blocks in the design auto& noc_ctx = g_vpr_ctx.noc(); // get a reference to the collection of router cluster blocks in the design const std::unordered_set& router_clusters = noc_ctx.noc_traffic_flows_storage.get_router_clusters_in_netlist(); @@ -533,7 +533,7 @@ e_create_move propose_router_swap(t_pl_blocks_to_be_moved& blocks_affected, floa e_create_move create_move = ::create_move(blocks_affected, b_from, to); - //Check that all of the blocks affected by the move would still be in a legal floorplan region after the swap + //Check that all the blocks affected by the move would still be in a legal floorplan region after the swap if (!floorplan_legal(blocks_affected)) { return e_create_move::ABORT; } From e56b21e5e07300fa7a2bacc2dafa67b482973f81 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 8 May 2023 18:10:44 -0400 Subject: [PATCH 03/29] simplified update_traffic_flow_link_usage (item 5 in issue 2262) --- vpr/src/place/noc_place_utils.cpp | 20 +++++--------------- vpr/src/place/noc_place_utils.h | 20 ++++---------------- 2 files changed, 9 insertions(+), 31 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index ae3c4e17ea1..ed63d9496d3 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -39,7 +39,7 @@ void initial_noc_placement(void) { std::vector& curr_traffic_flow_route = get_traffic_flow_route(conv_traffic_flow_id, noc_ctx.noc_model, *noc_traffic_flows_storage, noc_ctx.noc_flows_router, place_ctx.block_locs); // update the links used in the found traffic flow route - update_traffic_flow_link_usage(curr_traffic_flow_route, noc_ctx.noc_model, link_usage_update_state::increment, curr_traffic_flow.traffic_flow_bandwidth); + update_traffic_flow_link_usage(curr_traffic_flow_route, noc_ctx.noc_model, 1, curr_traffic_flow.traffic_flow_bandwidth); } return; @@ -123,24 +123,14 @@ std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, return curr_traffic_flow_route; } -void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, link_usage_update_state how_to_update_links, double traffic_flow_bandwidth) { +void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, int how_to_update_links, double traffic_flow_bandwidth) { // go through the links within the traffic flow route and update their bandwidth usage for (auto& link_in_route_id : traffic_flow_route) { // get the link to update and its current bandwdith NocLink& curr_link = noc_model.get_single_mutable_noc_link(link_in_route_id); double curr_link_bandwidth = curr_link.get_bandwidth_usage(); - // update the link badnwidth based on the user provided state - switch (how_to_update_links) { - case link_usage_update_state::increment: - curr_link.set_bandwidth_usage(curr_link_bandwidth + traffic_flow_bandwidth); - break; - case link_usage_update_state::decrement: - curr_link.set_bandwidth_usage(curr_link_bandwidth - traffic_flow_bandwidth); - break; - default: - break; - } + curr_link.set_bandwidth_usage(curr_link_bandwidth + how_to_update_links * traffic_flow_bandwidth); // check that the bandwidth never goes to negative VTR_ASSERT(curr_link.get_bandwidth_usage() >= 0.0); @@ -227,11 +217,11 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no * the existing traffic flow route */ const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage.get_traffic_flow_route(traffic_flow_id); - update_traffic_flow_link_usage(curr_traffic_flow_route, noc_model, link_usage_update_state::decrement, curr_traffic_flow.traffic_flow_bandwidth); + update_traffic_flow_link_usage(curr_traffic_flow_route, noc_model, -1 , curr_traffic_flow.traffic_flow_bandwidth); // now get the re-routed traffic flow route and update all the link usages with this reverted route std::vector& re_routed_traffic_flow_route = get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id, noc_model, noc_traffic_flows_storage, noc_flows_router, placed_cluster_block_locations); - update_traffic_flow_link_usage(re_routed_traffic_flow_route, noc_model, link_usage_update_state::increment, curr_traffic_flow.traffic_flow_bandwidth); + update_traffic_flow_link_usage(re_routed_traffic_flow_route, noc_model, 1, curr_traffic_flow.traffic_flow_bandwidth); return; } diff --git a/vpr/src/place/noc_place_utils.h b/vpr/src/place/noc_place_utils.h index 42500013169..12c776f45e8 100644 --- a/vpr/src/place/noc_place_utils.h +++ b/vpr/src/place/noc_place_utils.h @@ -31,20 +31,6 @@ struct NocPlaceStats { std::vector number_of_noc_router_moves_per_move_type; }; -/* Defines how the links found in a traffic flow are updated in terms - * of their bandwidth usage. - */ -enum class link_usage_update_state { - /* State 1: The link usages have to be incremented as the traffic - * flow route route has been updated - */ - increment, - /* State 2: The link usages have to be decremented as the traffic flow - * route is being removed - */ - decrement -}; - /** * @brief Routes all the traffic flows within the NoC and updates the link usage * for all links. This should be called after initial placement, where all the @@ -160,11 +146,13 @@ std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, * @param noc_model Contains all the links and routers within the NoC. Used * to update link information. * @param how_to_update_links Determines how the bandwidths of links found - * in the traffic flow route are updated. + * in the traffic flow route are updated. If it is -1, the route flow has + * been removed and links' bandwidth must be decremented. Otherwise, the a traffic + * flow has been re-routed and its links' bandwidth should be incremented. * @param traffic_flow_bandwidth The bandwidth of a traffic flow. This will * be used to update bandwidth usage of the links. */ -void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, link_usage_update_state how_to_update_links, double traffic_flow_bandwidth); +void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, int how_to_update_links, double traffic_flow_bandwidth); /** * @brief Goes through all the traffic flows associated to a moved From 0affc7468fe0dbe1f3f6b2fb6bfd2d9141b78c85 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 13:22:46 -0400 Subject: [PATCH 04/29] make format executed --- vpr/src/place/noc_place_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index ed63d9496d3..2991e56ce3d 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -217,7 +217,7 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no * the existing traffic flow route */ const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage.get_traffic_flow_route(traffic_flow_id); - update_traffic_flow_link_usage(curr_traffic_flow_route, noc_model, -1 , curr_traffic_flow.traffic_flow_bandwidth); + update_traffic_flow_link_usage(curr_traffic_flow_route, noc_model, -1, curr_traffic_flow.traffic_flow_bandwidth); // now get the re-routed traffic flow route and update all the link usages with this reverted route std::vector& re_routed_traffic_flow_route = get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id, noc_model, noc_traffic_flows_storage, noc_flows_router, placed_cluster_block_locations); From 9004a2a77ba78996753138d3bbde9f60856cee06 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 13:27:46 -0400 Subject: [PATCH 05/29] found a comment typo, update some comments --- vpr/src/base/vpr_context.h | 2 +- vpr/src/place/noc_place_utils.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/vpr/src/base/vpr_context.h b/vpr/src/base/vpr_context.h index ef595d22754..7a2d8450819 100644 --- a/vpr/src/base/vpr_context.h +++ b/vpr/src/base/vpr_context.h @@ -519,7 +519,7 @@ struct NocContext : public Context { /** * @brief Stores all the communication happening between routers in the NoC * - * Contains all of the traffic flows that ddescribe which pairs of logical routers are communicating and also some metrics and constraints on the data transfer between the two routers. + * Contains all of the traffic flows that describe which pairs of logical routers are communicating and also some metrics and constraints on the data transfer between the two routers. * * * This is created from a user supplied .flows file. diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 2991e56ce3d..baafc79c5ec 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -38,7 +38,7 @@ void initial_noc_placement(void) { // update the traffic flow route based on where the router cluster blocks are placed std::vector& curr_traffic_flow_route = get_traffic_flow_route(conv_traffic_flow_id, noc_ctx.noc_model, *noc_traffic_flows_storage, noc_ctx.noc_flows_router, place_ctx.block_locs); - // update the links used in the found traffic flow route + // update the links used in the found traffic flow route, links' bandwidth should be incremented since the traffic flow is routed update_traffic_flow_link_usage(curr_traffic_flow_route, noc_ctx.noc_model, 1, curr_traffic_flow.traffic_flow_bandwidth); } @@ -212,14 +212,14 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage.get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); /* since the current traffic flow route will be - * changed, first we need to reduce the bandwidh + * changed, first we need to decrement the bandwidth * usage of all links that are part of * the existing traffic flow route */ const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage.get_traffic_flow_route(traffic_flow_id); update_traffic_flow_link_usage(curr_traffic_flow_route, noc_model, -1, curr_traffic_flow.traffic_flow_bandwidth); - // now get the re-routed traffic flow route and update all the link usages with this reverted route + // now get the re-routed traffic flow route and increment all the link usages with this reverted route std::vector& re_routed_traffic_flow_route = get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id, noc_model, noc_traffic_flows_storage, noc_flows_router, placed_cluster_block_locations); update_traffic_flow_link_usage(re_routed_traffic_flow_route, noc_model, 1, curr_traffic_flow.traffic_flow_bandwidth); From d1a9bc034b3d34f7f309e80d12fc2a7ff08a4ddd Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 14:42:35 -0400 Subject: [PATCH 06/29] changed noc internal datastructure to reduce propose_router_swap time complexity (item 2 in issue 2262) --- vpr/src/noc/noc_traffic_flows.cpp | 14 ++++++++------ vpr/src/noc/noc_traffic_flows.h | 10 +++++++--- vpr/src/noc/read_xml_noc_traffic_flows_file.cpp | 3 +++ vpr/src/place/noc_place_utils.cpp | 9 +++------ 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index 9dd3565a149..b3d075be85c 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -44,7 +44,7 @@ std::vector& NocTrafficFlows::get_mutable_traffic_flow_route(NocTraff return traffic_flow_routes[traffic_flow_id]; } -const std::unordered_set& NocTrafficFlows::get_router_clusters_in_netlist(void) const { +const std::vector& NocTrafficFlows::get_router_clusters_in_netlist(void) const { return router_cluster_in_netlist; } @@ -63,14 +63,16 @@ void NocTrafficFlows::create_noc_traffic_flow(std::string source_router_module_n add_traffic_flow_to_associated_routers(curr_traffic_flow_id, source_router_cluster_id); add_traffic_flow_to_associated_routers(curr_traffic_flow_id, sink_router_cluster_id); - // insert the clusters to the local collection of all router clusters in the netlist - // duplicates should not be added multiple times - router_cluster_in_netlist.insert(source_router_cluster_id); - router_cluster_in_netlist.insert(sink_router_cluster_id); - return; } +//SARA_TODO: commenting +void NocTrafficFlows::set_router_cluster_in_netlist(std::vector routers_cluster_id_in_netlist){ + for (auto router_id : routers_cluster_id_in_netlist){ + router_cluster_in_netlist.emplace_back(router_id); + } +} + // utility functions for the noc traffic flows void NocTrafficFlows::finshed_noc_traffic_flows_setup(void) { diff --git a/vpr/src/noc/noc_traffic_flows.h b/vpr/src/noc/noc_traffic_flows.h index bdb104342b4..2fa384bd2ba 100644 --- a/vpr/src/noc/noc_traffic_flows.h +++ b/vpr/src/noc/noc_traffic_flows.h @@ -80,7 +80,7 @@ class NocTrafficFlows { vtr::vector noc_traffic_flows; /** contains the ids of all the router cluster blocks within the design */ - std::unordered_set router_cluster_in_netlist; + std::vector router_cluster_in_netlist; /** * @brief Each traffic flow is composed of a source and destination @@ -209,7 +209,7 @@ class NocTrafficFlows { */ std::vector& get_mutable_traffic_flow_route(NocTrafficFlowId traffic_flow_id); - const std::unordered_set& get_router_clusters_in_netlist(void) const; + const std::vector& get_router_clusters_in_netlist(void) const; // setters @@ -231,7 +231,7 @@ class NocTrafficFlows { * @param source_router_cluster_id The source router block id that * uniquely identifies this block in the clustered netlist. * @param sink_router_cluster_id The sink router block id that - * uniquely identifier this block in the clusterd netlist. + * uniquely identifier this block in the clustered netlist. * @param traffic_flow_bandwidth The size of the data transmission * in this traffic flow (units of bps). * @param traffic_flow_latency The maximum allowable delay between @@ -241,6 +241,9 @@ class NocTrafficFlows { */ void create_noc_traffic_flow(std::string source_router_module_name, std::string sink_router_module_name, ClusterBlockId source_router_cluster_id, ClusterBlockId sink_router_cluster_id, double traffic_flow_bandwidth, double traffic_flow_latency, int traffic_flow_priority); + + void set_router_cluster_in_netlist(std::vector routers_cluster_id_in_netlist); + //utility functions /** @@ -251,6 +254,7 @@ class NocTrafficFlows { * the routed paths for all traffic flows. * */ + void finshed_noc_traffic_flows_setup(void); /** diff --git a/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp b/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp index e5eb4894165..9e63581647b 100644 --- a/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp +++ b/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp @@ -55,6 +55,9 @@ void read_xml_noc_traffic_flows_file(const char* noc_flows_file) { process_single_flow(single_flow, loc_data, cluster_ctx, noc_ctx, noc_router_tile_type, cluster_blocks_compatible_with_noc_router_tiles); } + // insert the clusters to the local collection of all router clusters in the netlist + noc_ctx.noc_traffic_flows_storage.set_router_cluster_in_netlist(cluster_blocks_compatible_with_noc_router_tiles); + } catch (pugiutil::XmlError& e) { // used for identifying any of the xml parsing library errors vpr_throw(VPR_ERROR_OTHER, noc_flows_file, e.line(), e.what()); diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index baafc79c5ec..0e16f3e9971 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -482,7 +482,7 @@ e_create_move propose_router_swap(t_pl_blocks_to_be_moved& blocks_affected, floa // need to access all the router cluster blocks in the design auto& noc_ctx = g_vpr_ctx.noc(); // get a reference to the collection of router cluster blocks in the design - const std::unordered_set& router_clusters = noc_ctx.noc_traffic_flows_storage.get_router_clusters_in_netlist(); + const std::vector& router_clusters = noc_ctx.noc_traffic_flows_storage.get_router_clusters_in_netlist(); // if there are no router cluster blocks to swap then abort if (router_clusters.empty()) { @@ -496,10 +496,7 @@ e_create_move propose_router_swap(t_pl_blocks_to_be_moved& blocks_affected, floa * have iterated through the chosen random number of blocks. The cluster * we have stopped at will be the cluster to swap.*/ int random_cluster_block_index = vtr::irand(number_of_router_blocks - 1); - auto router_cluster_block_to_swap_ref = router_clusters.begin(); - std::advance(router_cluster_block_to_swap_ref, random_cluster_block_index); - - ClusterBlockId b_from = *router_cluster_block_to_swap_ref; + ClusterBlockId b_from = router_clusters[random_cluster_block_index]; auto& place_ctx = g_vpr_ctx.placement(); auto& cluster_ctx = g_vpr_ctx.clustering(); @@ -594,7 +591,7 @@ void write_noc_placement_file(std::string file_name) { int layer_number = 0; // get a reference to the collection of router cluster blocks in the design - const std::unordered_set& router_clusters = noc_ctx.noc_traffic_flows_storage.get_router_clusters_in_netlist(); + const std::vector& router_clusters = noc_ctx.noc_traffic_flows_storage.get_router_clusters_in_netlist(); //get the noc model to determine the physical routers where clusters are placed const NocStorage& noc_model = noc_ctx.noc_model; From 9d377c9eb11839c5113d6de35ee4813047371f64 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 14:51:07 -0400 Subject: [PATCH 07/29] commented the modified data structure, fixed some comments' typos --- vpr/src/noc/noc_traffic_flows.cpp | 2 +- vpr/src/noc/noc_traffic_flows.h | 22 ++++++++++++++----- .../noc/read_xml_noc_traffic_flows_file.cpp | 4 ++-- vpr/src/place/noc_place_utils.cpp | 5 +---- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index b3d075be85c..efcddae09f5 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -66,8 +66,8 @@ void NocTrafficFlows::create_noc_traffic_flow(std::string source_router_module_n return; } -//SARA_TODO: commenting void NocTrafficFlows::set_router_cluster_in_netlist(std::vector routers_cluster_id_in_netlist){ + //copy the input vector to the internal vector for (auto router_id : routers_cluster_id_in_netlist){ router_cluster_in_netlist.emplace_back(router_id); } diff --git a/vpr/src/noc/noc_traffic_flows.h b/vpr/src/noc/noc_traffic_flows.h index 2fa384bd2ba..fefa625a667 100644 --- a/vpr/src/noc/noc_traffic_flows.h +++ b/vpr/src/noc/noc_traffic_flows.h @@ -223,7 +223,7 @@ class NocTrafficFlows { * look up which traffic flows contain a specific router cluster block. * * @param source_router_module_name A string that represents the - * name of the source router block in the traffic flow. THis is + * name of the source router block in the traffic flow. This is * provided by the user. * @param sink_router_module_name A string that represents the name * of the sink router block in the traffic flow. This is provided by @@ -241,7 +241,19 @@ class NocTrafficFlows { */ void create_noc_traffic_flow(std::string source_router_module_name, std::string sink_router_module_name, ClusterBlockId source_router_cluster_id, ClusterBlockId sink_router_cluster_id, double traffic_flow_bandwidth, double traffic_flow_latency, int traffic_flow_priority); - + /** + * @brief fill "router_cluster_in_netlist" vector which is an internal + * data structure used in "propose_router_swap" function. This vector + * will contain NoC routers' ClusterBlockId specified in the netlist to + * simplify their quick access during choosing a random router from netlist. + * This routine is executed only once during traffic flow extraction + * after "get_cluster_blocks_compatible_with_noc_router_tiles" + * function and the vector will be constant afterward. + * + * @param routers_cluster_id_in_netlist A vector containing all routers' + * ClusterBlockId extracted from netlist. + * + */ void set_router_cluster_in_netlist(std::vector routers_cluster_id_in_netlist); //utility functions @@ -258,8 +270,8 @@ class NocTrafficFlows { void finshed_noc_traffic_flows_setup(void); /** - * @brief Resets the class by clearning internal - * satastructures. + * @brief Resets the class by clearing internal + * datastructures. * */ void clear_traffic_flows(void); @@ -267,7 +279,7 @@ class NocTrafficFlows { /** * @brief Given a block from the clustered netlist, determine * if the block has traffic flows that it is a part of. There are - * three posssible cases seen by this function. Case 1 is when the + * three possible cases seen by this function. Case 1 is when the * block is not a router. Case 2 is when the block is a router and * has not traffic flows it is a part of. And finally case three is * when the block is a router and has traffic flows it is a part of. diff --git a/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp b/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp index 9e63581647b..e6139b229ae 100644 --- a/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp +++ b/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp @@ -25,14 +25,14 @@ void read_xml_noc_traffic_flows_file(const char* noc_flows_file) { * which represents the name of the router modules in the HDL design. Each * time the cluster id is needed, the name of the block needs to be * compared to every block in the clustered netlist. This can be very - * time consuming, so instead we can compare to only blocks that are + * time-consuming, so instead we can compare to only blocks that are * compatible to physical NoC router tiles. */ std::vector cluster_blocks_compatible_with_noc_router_tiles = get_cluster_blocks_compatible_with_noc_router_tiles(cluster_ctx, noc_router_tile_type); /* variabled used when parsing the file. * Stores xml related information while parsing the file, such as current - * line number, current tag and etc. These variables will be used to + * line number, current tag and etc. These variables will be used to * provide additional information to the user when reporting an error. */ pugi::xml_document doc; diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 0e16f3e9971..14914c743d2 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -491,10 +491,7 @@ e_create_move propose_router_swap(t_pl_blocks_to_be_moved& blocks_affected, floa int number_of_router_blocks = router_clusters.size(); - /* We will choose a random number between 0-number_of_router_blocks-1. - * Then we will iterate through the router cluster blocks and stop when we - * have iterated through the chosen random number of blocks. The cluster - * we have stopped at will be the cluster to swap.*/ + //randomly choose a router block to move int random_cluster_block_index = vtr::irand(number_of_router_blocks - 1); ClusterBlockId b_from = router_clusters[random_cluster_block_index]; From b2172723738d051e0891ee1e28eef4ea1422c374 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 15:04:03 -0400 Subject: [PATCH 08/29] make format --- vpr/src/noc/noc_traffic_flows.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index efcddae09f5..08f0dc4ac34 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -66,9 +66,9 @@ void NocTrafficFlows::create_noc_traffic_flow(std::string source_router_module_n return; } -void NocTrafficFlows::set_router_cluster_in_netlist(std::vector routers_cluster_id_in_netlist){ +void NocTrafficFlows::set_router_cluster_in_netlist(std::vector routers_cluster_id_in_netlist) { //copy the input vector to the internal vector - for (auto router_id : routers_cluster_id_in_netlist){ + for (auto router_id : routers_cluster_id_in_netlist) { router_cluster_in_netlist.emplace_back(router_id); } } From 9a05c696bffa74adead024cb7ab25dbe5e8aed6c Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 15:41:23 -0400 Subject: [PATCH 09/29] removed extra casting --- vpr/src/noc/noc_traffic_flows.h | 2 +- vpr/src/place/noc_place_utils.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/vpr/src/noc/noc_traffic_flows.h b/vpr/src/noc/noc_traffic_flows.h index fefa625a667..a6b99937bd0 100644 --- a/vpr/src/noc/noc_traffic_flows.h +++ b/vpr/src/noc/noc_traffic_flows.h @@ -153,7 +153,7 @@ class NocTrafficFlows { * @brief Given a unique id of a traffic flow (t_noc_traffic_flow) * retrieve it from the vector of all traffic flows in the design. The * retrieved traffic flow cannot be modified but can be used to - * retireve information such as the routers involved. + * retrieve information such as the routers involved. * * @param traffic_flow_id The unique identifier (NocTrafficFlowId) * of the traffic flow to retrieve. diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 14914c743d2..995147a4399 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -19,7 +19,7 @@ void initial_noc_placement(void) { const auto& place_ctx = g_vpr_ctx.placement(); // need to update the link usages within after routing all the traffic flows - // also need to route all the traffic flows ans store them + // also need to route all the traffic flows and store them auto& noc_ctx = g_vpr_ctx.mutable_noc(); NocTrafficFlows* noc_traffic_flows_storage = &noc_ctx.noc_traffic_flows_storage; @@ -197,7 +197,7 @@ void revert_noc_traffic_flow_routes(const t_pl_blocks_to_be_moved& blocks_affect re_route_traffic_flow(traffic_flow_id, *noc_traffic_flows_storage, noc_ctx.noc_model, noc_ctx.noc_flows_router, place_ctx.block_locs); // make sure we do not revert this traffic flow again - reverted_traffic_flows.insert((NocTrafficFlowId)traffic_flow_id); + reverted_traffic_flows.insert(traffic_flow_id); } } } @@ -209,7 +209,7 @@ void revert_noc_traffic_flow_routes(const t_pl_blocks_to_be_moved& blocks_affect void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting* noc_flows_router, const vtr::vector_map& placed_cluster_block_locations) { // get the current traffic flow info - const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage.get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); + const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage.get_single_noc_traffic_flow(traffic_flow_id); /* since the current traffic flow route will be * changed, first we need to decrement the bandwidth @@ -220,7 +220,7 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no update_traffic_flow_link_usage(curr_traffic_flow_route, noc_model, -1, curr_traffic_flow.traffic_flow_bandwidth); // now get the re-routed traffic flow route and increment all the link usages with this reverted route - std::vector& re_routed_traffic_flow_route = get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id, noc_model, noc_traffic_flows_storage, noc_flows_router, placed_cluster_block_locations); + std::vector& re_routed_traffic_flow_route = get_traffic_flow_route(traffic_flow_id, noc_model, noc_traffic_flows_storage, noc_flows_router, placed_cluster_block_locations); update_traffic_flow_link_usage(re_routed_traffic_flow_route, noc_model, 1, curr_traffic_flow.traffic_flow_bandwidth); return; @@ -299,7 +299,7 @@ double comp_noc_latency_cost(const t_noc_opts& noc_opts) { // store the calculated latency for the current traffic flow in local datastructures (this also initializes them) traffic_flow_latency_cost[(NocTrafficFlowId)traffic_flow_id] = curr_traffic_flow_latency_cost; - // accumumulate the aggregate bandwidth cost + // accumulate the aggregate bandwidth cost noc_latency_cost += curr_traffic_flow_latency_cost; } From c1a840459296d510f8c02e813143067a7451d440 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 16:42:10 -0400 Subject: [PATCH 10/29] created a struct for each traffic flow agg_bandwidth and latency (item 3 in issue 2262) --- vpr/src/place/noc_place_utils.cpp | 45 ++++++++++++++----------------- vpr/src/place/noc_place_utils.h | 12 +++++++++ 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 995147a4399..c937390a57e 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -2,9 +2,8 @@ #include "noc_place_utils.h" /********************** Variables local to noc_place_utils.c pp***************************/ -/* Cost of a noc traffic flow, and a temporary cost of a noc traffic flow - * during move assessment*/ -static vtr::vector traffic_flow_aggregate_bandwidth_cost, proposed_traffic_flow_aggregate_bandwidth_cost, traffic_flow_latency_cost, proposed_traffic_flow_latency_cost; +/* Proposed and actual cost of a noc traffic flow used for each move assessment */ +static vtr::vector traffic_flow_costs, proposed_traffic_flow_costs; /* Keeps track of traffic flows that have been updated at each attempted placement move*/ static std::vector affected_traffic_flows; @@ -78,11 +77,11 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved // get the current traffic flow info const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); - proposed_traffic_flow_aggregate_bandwidth_cost[traffic_flow_id] = calculate_traffic_flow_aggregate_bandwidth_cost(traffic_flow_route, curr_traffic_flow); - proposed_traffic_flow_latency_cost[traffic_flow_id] = calculate_traffic_flow_latency_cost(traffic_flow_route, noc_ctx.noc_model, curr_traffic_flow, noc_opts); + proposed_traffic_flow_costs[traffic_flow_id].aggregate_bandwidth = calculate_traffic_flow_aggregate_bandwidth_cost(traffic_flow_route, curr_traffic_flow); + proposed_traffic_flow_costs[traffic_flow_id].latency = calculate_traffic_flow_latency_cost(traffic_flow_route, noc_ctx.noc_model, curr_traffic_flow, noc_opts); - noc_aggregate_bandwidth_delta_c += proposed_traffic_flow_aggregate_bandwidth_cost[traffic_flow_id] - traffic_flow_aggregate_bandwidth_cost[traffic_flow_id]; - noc_latency_delta_c += proposed_traffic_flow_latency_cost[traffic_flow_id] - traffic_flow_latency_cost[traffic_flow_id]; + noc_aggregate_bandwidth_delta_c += proposed_traffic_flow_costs[traffic_flow_id].aggregate_bandwidth - traffic_flow_costs[traffic_flow_id].aggregate_bandwidth; + noc_latency_delta_c += proposed_traffic_flow_costs[traffic_flow_id].latency - traffic_flow_costs[traffic_flow_id].latency; } return number_of_affected_traffic_flows; @@ -93,12 +92,12 @@ void commit_noc_costs(int number_of_affected_traffic_flows) { NocTrafficFlowId traffic_flow_id = affected_traffic_flows[traffic_flow_affected]; // update the traffic flow costs - traffic_flow_aggregate_bandwidth_cost[traffic_flow_id] = proposed_traffic_flow_aggregate_bandwidth_cost[traffic_flow_id]; - traffic_flow_latency_cost[traffic_flow_id] = proposed_traffic_flow_latency_cost[traffic_flow_id]; + traffic_flow_costs[traffic_flow_id] = proposed_traffic_flow_costs[traffic_flow_id]; // reset the proposed traffic flows costs - proposed_traffic_flow_aggregate_bandwidth_cost[traffic_flow_id] = -1; - proposed_traffic_flow_latency_cost[traffic_flow_id] = -1; + proposed_traffic_flow_costs[traffic_flow_id].aggregate_bandwidth = -1; + proposed_traffic_flow_costs[traffic_flow_id].latency = -1; + } return; @@ -235,8 +234,8 @@ void recompute_noc_costs(double* new_noc_aggregate_bandwidth_cost, double* new_n // go through the costs of all the traffic flows and add them up to recompute the total costs associated with the NoC for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { - *new_noc_aggregate_bandwidth_cost += traffic_flow_aggregate_bandwidth_cost[(NocTrafficFlowId)traffic_flow_id]; - *new_noc_latency_cost += traffic_flow_latency_cost[(NocTrafficFlowId)traffic_flow_id]; + *new_noc_aggregate_bandwidth_cost += traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].aggregate_bandwidth; + *new_noc_latency_cost += traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].latency; } return; @@ -269,9 +268,9 @@ double comp_noc_aggregate_bandwidth_cost(void) { double curr_traffic_flow_aggregate_bandwidth_cost = calculate_traffic_flow_aggregate_bandwidth_cost(curr_traffic_flow_route, curr_traffic_flow); // store the calculated aggregate bandwidth for the current traffic flow in local datastructures (this also initializes them) - traffic_flow_aggregate_bandwidth_cost[(NocTrafficFlowId)traffic_flow_id] = curr_traffic_flow_aggregate_bandwidth_cost; + traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].aggregate_bandwidth = curr_traffic_flow_aggregate_bandwidth_cost; - // accumumulate the aggregate bandwidth cost + // accumulate the aggregate bandwidth cost noc_aggregate_bandwidth_cost += curr_traffic_flow_aggregate_bandwidth_cost; } @@ -297,7 +296,7 @@ double comp_noc_latency_cost(const t_noc_opts& noc_opts) { double curr_traffic_flow_latency_cost = calculate_traffic_flow_latency_cost(curr_traffic_flow_route, noc_ctx.noc_model, curr_traffic_flow, noc_opts); // store the calculated latency for the current traffic flow in local datastructures (this also initializes them) - traffic_flow_latency_cost[(NocTrafficFlowId)traffic_flow_id] = curr_traffic_flow_latency_cost; + traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].latency = curr_traffic_flow_latency_cost; // accumulate the aggregate bandwidth cost noc_latency_cost += curr_traffic_flow_latency_cost; @@ -351,7 +350,7 @@ int check_noc_placement_costs(const t_placer_costs& costs, double error_toleranc double current_flow_latency_cost = calculate_traffic_flow_latency_cost(temp_found_noc_route, *noc_model, curr_traffic_flow, noc_opts); noc_latency_cost_check += current_flow_latency_cost; - // clear the current traffic flow route so we can route the next traffic flow + // clear the current traffic flow route, so we can route the next traffic flow temp_found_noc_route.clear(); } @@ -447,10 +446,8 @@ void allocate_and_load_noc_placement_structs(void) { int number_of_traffic_flows = noc_ctx.noc_traffic_flows_storage.get_number_of_traffic_flows(); - traffic_flow_aggregate_bandwidth_cost.resize(number_of_traffic_flows, -1); - proposed_traffic_flow_aggregate_bandwidth_cost.resize(number_of_traffic_flows, -1); - traffic_flow_latency_cost.resize(number_of_traffic_flows, -1); - proposed_traffic_flow_latency_cost.resize(number_of_traffic_flows, -1); + traffic_flow_costs.resize(number_of_traffic_flows); + proposed_traffic_flow_costs.resize(number_of_traffic_flows); affected_traffic_flows.resize(number_of_traffic_flows, NocTrafficFlowId::INVALID()); @@ -458,10 +455,8 @@ void allocate_and_load_noc_placement_structs(void) { } void free_noc_placement_structs(void) { - vtr::release_memory(traffic_flow_aggregate_bandwidth_cost); - vtr::release_memory(proposed_traffic_flow_aggregate_bandwidth_cost); - vtr::release_memory(traffic_flow_latency_cost); - vtr::release_memory(proposed_traffic_flow_latency_cost); + vtr::release_memory(traffic_flow_costs); + vtr::release_memory(proposed_traffic_flow_costs); vtr::release_memory(affected_traffic_flows); return; diff --git a/vpr/src/place/noc_place_utils.h b/vpr/src/place/noc_place_utils.h index 12c776f45e8..225368ea94c 100644 --- a/vpr/src/place/noc_place_utils.h +++ b/vpr/src/place/noc_place_utils.h @@ -31,6 +31,18 @@ struct NocPlaceStats { std::vector number_of_noc_router_moves_per_move_type; }; +/** + * @brief Each traffic flow cost consists of two components: + * 1) traffic flow aggregate bandwidth + * 2) traffic flow latency + * NoC placement code will keep an array-of-struct to easily access each + * traffic flow cost. + */ +struct TrafficFlowPlaceCost { + double aggregate_bandwidth = -1; + double latency = -1; +}; + /** * @brief Routes all the traffic flows within the NoC and updates the link usage * for all links. This should be called after initial placement, where all the From efd718e780bbd6e4c4b89cfb6c64a534fb282a48 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 16:42:54 -0400 Subject: [PATCH 11/29] C++ formatting --- vpr/src/place/noc_place_utils.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index c937390a57e..d063c27a5bf 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -3,7 +3,7 @@ /********************** Variables local to noc_place_utils.c pp***************************/ /* Proposed and actual cost of a noc traffic flow used for each move assessment */ -static vtr::vector traffic_flow_costs, proposed_traffic_flow_costs; +static vtr::vector traffic_flow_costs, proposed_traffic_flow_costs; /* Keeps track of traffic flows that have been updated at each attempted placement move*/ static std::vector affected_traffic_flows; @@ -97,7 +97,6 @@ void commit_noc_costs(int number_of_affected_traffic_flows) { // reset the proposed traffic flows costs proposed_traffic_flow_costs[traffic_flow_id].aggregate_bandwidth = -1; proposed_traffic_flow_costs[traffic_flow_id].latency = -1; - } return; From a11dc3b2278da825a34189435fd72fad2d5cda2b Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 17:16:23 -0400 Subject: [PATCH 12/29] changed function arguments from pointer ro ref (item 6 in issue 2262) --- vpr/src/noc/noc_storage.cpp | 2 +- vpr/src/place/noc_place_utils.cpp | 24 ++++++++++++------------ vpr/src/place/noc_place_utils.h | 26 +++++++++++++------------- vpr/src/place/place.cpp | 2 +- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/vpr/src/noc/noc_storage.cpp b/vpr/src/noc/noc_storage.cpp index 9c380fc27e6..1703d4dac5d 100644 --- a/vpr/src/noc/noc_storage.cpp +++ b/vpr/src/noc/noc_storage.cpp @@ -140,7 +140,7 @@ bool NocStorage::remove_link(NocRouterId src_router_id, NocRouterId sink_router_ for (auto outgoing_link_id = source_router_outgoing_links->begin(); outgoing_link_id != source_router_outgoing_links->end(); outgoing_link_id++) { // check to see if the current link id matches the id of the link to remove if (link_storage[*outgoing_link_id].get_sink_router() == sink_router_id) { - // found the link we need to remove so we delete it here + // found the link we need to remove, so we delete it here //change the link to be invalid link_storage[*outgoing_link_id].set_source_router(NocRouterId::INVALID()); link_storage[*outgoing_link_id].set_sink_router(NocRouterId::INVALID()); diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index d063c27a5bf..95babf4d4a8 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -35,7 +35,7 @@ void initial_noc_placement(void) { const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(conv_traffic_flow_id); // update the traffic flow route based on where the router cluster blocks are placed - std::vector& curr_traffic_flow_route = get_traffic_flow_route(conv_traffic_flow_id, noc_ctx.noc_model, *noc_traffic_flows_storage, noc_ctx.noc_flows_router, place_ctx.block_locs); + std::vector& curr_traffic_flow_route = get_traffic_flow_route(conv_traffic_flow_id, noc_ctx.noc_model, *noc_traffic_flows_storage, *noc_ctx.noc_flows_router, place_ctx.block_locs); // update the links used in the found traffic flow route, links' bandwidth should be incremented since the traffic flow is routed update_traffic_flow_link_usage(curr_traffic_flow_route, noc_ctx.noc_model, 1, curr_traffic_flow.traffic_flow_bandwidth); @@ -64,7 +64,7 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved // check if the current moved block is a noc router if (noc_traffic_flows_storage->check_if_cluster_block_has_traffic_flows(blk)) { // current block is a router, so re-route all the traffic flows it is a part of - re_route_associated_traffic_flows(blk, *noc_traffic_flows_storage, noc_ctx.noc_model, noc_ctx.noc_flows_router, place_ctx.block_locs, updated_traffic_flows, number_of_affected_traffic_flows); + re_route_associated_traffic_flows(blk, *noc_traffic_flows_storage, noc_ctx.noc_model, *noc_ctx.noc_flows_router, place_ctx.block_locs, updated_traffic_flows, number_of_affected_traffic_flows); } } @@ -102,7 +102,7 @@ void commit_noc_costs(int number_of_affected_traffic_flows) { return; } -std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, const NocStorage& noc_model, NocTrafficFlows& noc_traffic_flows_storage, NocRouting* noc_flows_router, const vtr::vector_map& placed_cluster_block_locations) { +std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, const NocStorage& noc_model, NocTrafficFlows& noc_traffic_flows_storage, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations) { // get the traffic flow with the current id const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage.get_single_noc_traffic_flow(traffic_flow_id); @@ -116,7 +116,7 @@ std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, // route the current traffic flow std::vector& curr_traffic_flow_route = noc_traffic_flows_storage.get_mutable_traffic_flow_route(traffic_flow_id); - noc_flows_router->route_flow(source_router_block_id, sink_router_block_id, curr_traffic_flow_route, noc_model); + noc_flows_router.route_flow(source_router_block_id, sink_router_block_id, curr_traffic_flow_route, noc_model); return curr_traffic_flow_route; } @@ -137,7 +137,7 @@ void update_traffic_flow_link_usage(const std::vector& traffic_flow_r return; } -void re_route_associated_traffic_flows(ClusterBlockId moved_block_router_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting* noc_flows_router, const vtr::vector_map& placed_cluster_block_locations, std::unordered_set& updated_traffic_flows, int& number_of_affected_traffic_flows) { +void re_route_associated_traffic_flows(ClusterBlockId moved_block_router_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations, std::unordered_set& updated_traffic_flows, int& number_of_affected_traffic_flows) { // get all the associated traffic flows for the logical router cluster block const std::vector* assoc_traffic_flows = noc_traffic_flows_storage.get_traffic_flows_associated_to_router_block(moved_block_router_id); @@ -192,7 +192,7 @@ void revert_noc_traffic_flow_routes(const t_pl_blocks_to_be_moved& blocks_affect // first check to see whether we have already reverted the current traffic flow and only revert it if we haven't already. if (reverted_traffic_flows.find(traffic_flow_id) == reverted_traffic_flows.end()) { // Revert the traffic flow route by re-routing it - re_route_traffic_flow(traffic_flow_id, *noc_traffic_flows_storage, noc_ctx.noc_model, noc_ctx.noc_flows_router, place_ctx.block_locs); + re_route_traffic_flow(traffic_flow_id, *noc_traffic_flows_storage, noc_ctx.noc_model, *noc_ctx.noc_flows_router, place_ctx.block_locs); // make sure we do not revert this traffic flow again reverted_traffic_flows.insert(traffic_flow_id); @@ -205,7 +205,7 @@ void revert_noc_traffic_flow_routes(const t_pl_blocks_to_be_moved& blocks_affect return; } -void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting* noc_flows_router, const vtr::vector_map& placed_cluster_block_locations) { +void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations) { // get the current traffic flow info const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage.get_single_noc_traffic_flow(traffic_flow_id); @@ -224,17 +224,17 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no return; } -void recompute_noc_costs(double* new_noc_aggregate_bandwidth_cost, double* new_noc_latency_cost) { +void recompute_noc_costs(double& new_noc_aggregate_bandwidth_cost, double& new_noc_latency_cost) { int number_of_traffic_flows = g_vpr_ctx.noc().noc_traffic_flows_storage.get_number_of_traffic_flows(); // reset the cost variables first - *new_noc_aggregate_bandwidth_cost = 0; - *new_noc_latency_cost = 0; + new_noc_aggregate_bandwidth_cost = 0; + new_noc_latency_cost = 0; // go through the costs of all the traffic flows and add them up to recompute the total costs associated with the NoC for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { - *new_noc_aggregate_bandwidth_cost += traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].aggregate_bandwidth; - *new_noc_latency_cost += traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].latency; + new_noc_aggregate_bandwidth_cost += traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].aggregate_bandwidth; + new_noc_latency_cost += traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].latency; } return; diff --git a/vpr/src/place/noc_place_utils.h b/vpr/src/place/noc_place_utils.h index 225368ea94c..668a1469650 100644 --- a/vpr/src/place/noc_place_utils.h +++ b/vpr/src/place/noc_place_utils.h @@ -142,7 +142,7 @@ void commit_noc_costs(int number_of_affected_traffic_flows); * placed grid locations of all cluster blocks. * @return std::vector& The found route for the traffic flow. */ -std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, const NocStorage& noc_model, NocTrafficFlows& noc_traffic_flows_storage, NocRouting* noc_flows_router, const vtr::vector_map& placed_cluster_block_locations); +std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, const NocStorage& noc_model, NocTrafficFlows& noc_traffic_flows_storage, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations); /** * @brief Updates the bandwidth usages of links found in a routed traffic flow. @@ -190,13 +190,13 @@ void update_traffic_flow_link_usage(const std::vector& traffic_flow_r * @param placed_cluster_block_locations A datastructure that identifies the * placed grid locations of all cluster blocks. * @param updated_traffic_flows Keeps track of traffic flows that have been - * re-routed. Used to prevent re-rouitng the same traffic flow multiple times. + * re-routed. Used to prevent re-routing the same traffic flow multiple times. * @param number_of_affected_traffic_flows an integer which represents the * number of traffic flows which were affected (modified) due to either * their source or destination routers being moved for a single placement * iteration. This is updated within this function. */ -void re_route_associated_traffic_flows(ClusterBlockId moved_router_block_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting* noc_flows_router, const vtr::vector_map& placed_cluster_block_locations, std::unordered_set& updated_traffic_flows, int& number_of_affected_traffic_flows); +void re_route_associated_traffic_flows(ClusterBlockId moved_router_block_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations, std::unordered_set& updated_traffic_flows, int& number_of_affected_traffic_flows); /** * @brief Used to re-route all the traffic flows associated to logical @@ -229,7 +229,7 @@ void revert_noc_traffic_flow_routes(const t_pl_blocks_to_be_moved& blocks_affect * @param placed_cluster_block_locations A datastructure that identifies the * placed grid locations of all cluster blocks. */ -void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting* noc_flows_router, const vtr::vector_map& placed_cluster_block_locations); +void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations); /** * @brief Recompute the NoC costs (aggregate bandwidth and latency) by @@ -243,14 +243,14 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no * the incremental changes to the NoC costs are correct by comparing * the current costs to newly computed costs for the current * placement state. This function assumes the traffic flows have - * been routed and all the inidividual NoC costs of all traffic flows are + * been routed and all the individual NoC costs of all traffic flows are * correct and only accumulates them to compute the new overall NoC costs. * * If the comparison is larger than the error tolerance then it * implies that the incremental cost updates were incorrect and * an error is thrown. * - * This function is not very expensive and can be called regularily during + * This function is not very expensive and can be called regularly during * placement to ensure the NoC costs do not deviate too far off * from their correct values. * @@ -259,7 +259,7 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no * @param new_noc_latency_cost Will store the newly computed * NoC latency cost for the current placement state. */ -void recompute_noc_costs(double* new_noc_aggregate_bandwidth_cost, double* new_noc_latency_cost); +void recompute_noc_costs(double& new_noc_aggregate_bandwidth_cost, double& new_noc_latency_cost); /** * @brief Updates all the cost normalization factors relevant to the NoC. @@ -267,7 +267,7 @@ void recompute_noc_costs(double* new_noc_aggregate_bandwidth_cost, double* new_n * reach INF. * This is intended to be used to initialize the normalization factors of * the NoC and also at the outer loop iteration of placement to - * balance the NoC costs with other placment cost parameters. + * balance the NoC costs with other placement cost parameters. * * @param costs Contains the normalization factors which need to be updated */ @@ -281,7 +281,7 @@ void update_noc_normalization_factors(t_placer_costs& costs); * the individual traffic flow aggregate bandwidths. * * This should be used after initial placement to determine the starting - * aggregate bandwdith cost of the NoC. + * aggregate bandwidth cost of the NoC. * * @return double The aggregate bandwidth cost of the NoC. */ @@ -324,9 +324,9 @@ double comp_noc_latency_cost(const t_noc_opts& noc_opts); * placement. * @param error_tolerance The maximum allowable difference between the current * NoC costs and the newly computed NoC costs. - * @param noc_opts Contains information neccessary to compute the NoC costs + * @param noc_opts Contains information necessary to compute the NoC costs * from scratch. For example this would include the routing algorithm and - * weights for the differnce components of the NoC costs. + * weights for the difference components of the NoC costs. * @return An integer which represents the status of the comparison. 0 * indicates that the current NoC costs are within the error tolerance and * a non-zero values indicates the current NoC costs are above the error @@ -382,7 +382,7 @@ int get_number_of_traffic_flows_with_latency_cons_met(void); * store the status of the NoC placement after each move. We create and * initialize the datastructures here. * - * This should be called before starting the simulated annealing palcement. + * This should be called before starting the simulated annealing placement. * */ void allocate_and_load_noc_placement_structs(void); @@ -423,7 +423,7 @@ bool check_for_router_swap(int user_supplied_noc_router_swap_percentage); */ e_create_move propose_router_swap(t_pl_blocks_to_be_moved& blocks_affected, float rlim); -/* Below are functions related to modifying and retreiving the NoC placement stats dastructure*/ +/* Below are functions related to modifying and retrieving the NoC placement stats data structure*/ /** * @brief Initializes all the stat values to 0 and allocates space for the diff --git a/vpr/src/place/place.cpp b/vpr/src/place/place.cpp index 5a1f1295f76..1841c6191fd 100644 --- a/vpr/src/place/place.cpp +++ b/vpr/src/place/place.cpp @@ -1231,7 +1231,7 @@ static void recompute_costs_from_scratch(const t_placer_opts& placer_opts, if (noc_opts.noc) { double new_noc_aggregate_bandwidth_cost = 0.; double new_noc_latency_cost = 0.; - recompute_noc_costs(&new_noc_aggregate_bandwidth_cost, &new_noc_latency_cost); + recompute_noc_costs(new_noc_aggregate_bandwidth_cost, new_noc_latency_cost); if (fabs( new_noc_aggregate_bandwidth_cost From 8937c914246fb15c0f9e910af11b0a2a33762525 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 11 May 2023 18:08:27 -0400 Subject: [PATCH 13/29] fixed test_noc_traffic_flow.cpp to work with new function params --- vpr/test/test_noc_place_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vpr/test/test_noc_place_utils.cpp b/vpr/test/test_noc_place_utils.cpp index e58a10f6549..3ab03f92fd7 100644 --- a/vpr/test/test_noc_place_utils.cpp +++ b/vpr/test/test_noc_place_utils.cpp @@ -1039,7 +1039,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ test_noc_latency_costs = 0.; // now execute the test function - recompute_noc_costs(&test_noc_bandwidth_costs, &test_noc_latency_costs); + recompute_noc_costs(test_noc_bandwidth_costs, test_noc_latency_costs); // now verify REQUIRE(vtr::isclose(golden_total_noc_latency_cost, test_noc_latency_costs)); From 95aa796dc36130fab866cb63d3f9906403c30f18 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 10:47:59 -0400 Subject: [PATCH 14/29] removed some extra casting between loop variable and NoCTrafficFlowId by using range-based loop for affected_traffic_flow data structure (item 4 in issue 2262) --- vpr/src/place/noc_place_utils.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index c2de2328f06..13d43c02bcc 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -52,6 +52,7 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved std::unordered_set updated_traffic_flows; int number_of_affected_traffic_flows = 0; + affected_traffic_flows.clear(); // go through the moved blocks and process them only if they are NoC routers for (int iblk = 0; iblk < blocks_affected.num_moved_blocks; ++iblk) { @@ -65,8 +66,7 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved } // go through all the affected traffic flows and calculate their new costs after being re-routed, then determine the change in cost before the traffic flows were modified - for (int traffic_flow_affected = 0; traffic_flow_affected < number_of_affected_traffic_flows; traffic_flow_affected++) { - NocTrafficFlowId traffic_flow_id = affected_traffic_flows[traffic_flow_affected]; + for (auto& traffic_flow_id : affected_traffic_flows) { // get the traffic flow route const std::vector& traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route(traffic_flow_id); @@ -84,9 +84,7 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved } void commit_noc_costs(int number_of_affected_traffic_flows) { - for (int traffic_flow_affected = 0; traffic_flow_affected < number_of_affected_traffic_flows; traffic_flow_affected++) { - NocTrafficFlowId traffic_flow_id = affected_traffic_flows[traffic_flow_affected]; - + for (auto& traffic_flow_id : affected_traffic_flows){ // update the traffic flow costs traffic_flow_costs[traffic_flow_id] = proposed_traffic_flow_costs[traffic_flow_id]; @@ -150,7 +148,7 @@ void re_route_associated_traffic_flows(ClusterBlockId moved_block_router_id, Noc updated_traffic_flows.insert(traffic_flow_id); // update global datastructures to indicate that the current traffic flow was affected due to router cluster blocks being swapped - affected_traffic_flows[number_of_affected_traffic_flows] = traffic_flow_id; + affected_traffic_flows.push_back(traffic_flow_id); number_of_affected_traffic_flows++; } } @@ -255,7 +253,7 @@ double comp_noc_aggregate_bandwidth_cost(void) { double noc_aggregate_bandwidth_cost = 0.; // now go through each traffic flow route and calculate its - // aggregate bandwidth. Then store this in local datastrucutres and accumulate it. + // aggregate bandwidth. Then store this in local data structures and accumulate it. for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id); @@ -444,8 +442,6 @@ void allocate_and_load_noc_placement_structs(void) { traffic_flow_costs.resize(number_of_traffic_flows); proposed_traffic_flow_costs.resize(number_of_traffic_flows); - affected_traffic_flows.resize(number_of_traffic_flows, NocTrafficFlowId::INVALID()); - return; } From 3c70337dd2b0384d9f852e4b860f0d24f23b6c6c Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 10:52:46 -0400 Subject: [PATCH 15/29] removed extra function argument from commit_noc_costs --- vpr/src/place/noc_place_utils.cpp | 2 +- vpr/src/place/noc_place_utils.h | 9 ++------- vpr/src/place/place.cpp | 2 +- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 13d43c02bcc..b250c43379d 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -83,7 +83,7 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved return number_of_affected_traffic_flows; } -void commit_noc_costs(int number_of_affected_traffic_flows) { +void commit_noc_costs() { for (auto& traffic_flow_id : affected_traffic_flows){ // update the traffic flow costs traffic_flow_costs[traffic_flow_id] = proposed_traffic_flow_costs[traffic_flow_id]; diff --git a/vpr/src/place/noc_place_utils.h b/vpr/src/place/noc_place_utils.h index 77e96c2c8e5..d3474e176b4 100644 --- a/vpr/src/place/noc_place_utils.h +++ b/vpr/src/place/noc_place_utils.h @@ -105,15 +105,10 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved * This function should be used after a proposed move which includes NoC * router blocks (logical) is accepted. The move needs to be accepted * since the affected traffic flow costs are updated here to reflect the - * current placement and the placmennt is only updated when a move is + * current placement and the placement is only updated when a move is * accepted. - * - * @param number_of_affected_traffic_flows An integer which represents the - * number of traffic flows which were affected (modified) due to either - * their source or destination routers being moved for a single placement - * iteration. */ -void commit_noc_costs(int number_of_affected_traffic_flows); +void commit_noc_costs(); /** * @brief Routes a given traffic flow within the NoC based on where the diff --git a/vpr/src/place/place.cpp b/vpr/src/place/place.cpp index 207ad4e52a3..a55f2b51a30 100644 --- a/vpr/src/place/place.cpp +++ b/vpr/src/place/place.cpp @@ -1612,7 +1612,7 @@ static e_move_result try_swap(const t_annealing_state* state, ++move_type_stat.accepted_moves[(move_blk_type.index * (placer_opts.place_static_move_prob.size())) + (int)move_type]; } if (noc_opts.noc) { - commit_noc_costs(number_of_affected_noc_traffic_flows); + commit_noc_costs(); costs->noc_aggregate_bandwidth_cost += noc_aggregate_bandwidth_delta_c; costs->noc_latency_cost += noc_latency_delta_c; From a50c9a49ba0ba904caf32eb648518ede92d46583 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 10:59:54 -0400 Subject: [PATCH 16/29] removed extra conversion --- vpr/src/place/noc_place_utils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index b250c43379d..acbf8d6fd40 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -71,7 +71,7 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved const std::vector& traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route(traffic_flow_id); // get the current traffic flow info - const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); + const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); proposed_traffic_flow_costs[traffic_flow_id].aggregate_bandwidth = calculate_traffic_flow_aggregate_bandwidth_cost(traffic_flow_route, curr_traffic_flow); proposed_traffic_flow_costs[traffic_flow_id].latency = calculate_traffic_flow_latency_cost(traffic_flow_route, noc_ctx.noc_model, curr_traffic_flow, noc_opts); @@ -281,7 +281,7 @@ double comp_noc_latency_cost(const t_noc_opts& noc_opts) { double noc_latency_cost = 0.; // now go through each traffic flow route and calculate its - // latency. Then store this in local datastrucutres and accumulate it. + // latency. Then store this in local data structures and accumulate it. for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id); From a9734ef5b77b9d20f143ee2cae6fe1ec46788322 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 12:02:59 -0400 Subject: [PATCH 17/29] fixed all unnecessary casting in NoC-related code (item 4 in issue 2262) --- vpr/src/noc/noc_traffic_flows.cpp | 5 +++ vpr/src/noc/noc_traffic_flows.h | 7 ++++- vpr/src/place/noc_place_utils.cpp | 51 +++++++++++++------------------ 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index 08f0dc4ac34..3afa8d39d14 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -48,6 +48,10 @@ const std::vector& NocTrafficFlows::get_router_clusters_in_netli return router_cluster_in_netlist; } +const std::vector& NocTrafficFlows::get_all_traffic_flow_id(void) const{ + return noc_traffic_flows_ids; +} + // setters for the traffic flows void NocTrafficFlows::create_noc_traffic_flow(std::string source_router_module_name, std::string sink_router_module_name, ClusterBlockId source_router_cluster_id, ClusterBlockId sink_router_cluster_id, double traffic_flow_bandwidth, double traffic_flow_latency, int traffic_flow_priority) { @@ -58,6 +62,7 @@ void NocTrafficFlows::create_noc_traffic_flow(std::string source_router_module_n //since the new traffic flow was added to the back of the vector, its id will be the index of the last element NocTrafficFlowId curr_traffic_flow_id = (NocTrafficFlowId)(noc_traffic_flows.size() - 1); + noc_traffic_flows_ids.emplace_back(curr_traffic_flow_id); // now add the new traffic flow to flows associated with the current source and sink router add_traffic_flow_to_associated_routers(curr_traffic_flow_id, source_router_cluster_id); diff --git a/vpr/src/noc/noc_traffic_flows.h b/vpr/src/noc/noc_traffic_flows.h index a6b99937bd0..e3891ddf775 100644 --- a/vpr/src/noc/noc_traffic_flows.h +++ b/vpr/src/noc/noc_traffic_flows.h @@ -79,6 +79,9 @@ class NocTrafficFlows { /** contains all the traffic flows provided by the user and their information*/ vtr::vector noc_traffic_flows; + /** contains all the traffic flows ids provided by the user*/ + std::vector noc_traffic_flows_ids; + /** contains the ids of all the router cluster blocks within the design */ std::vector router_cluster_in_netlist; @@ -88,7 +91,7 @@ class NocTrafficFlows { * flow needs tp be re-routed. * * This datastructure stores a vector of traffic flows that are associated - * to each router cbluster block. A traffic flow is associated to a router + * to each router cluster block. A traffic flow is associated to a router * cluster block if the router block is either the source or destination * router within the traffic flow. * @@ -211,6 +214,8 @@ class NocTrafficFlows { const std::vector& get_router_clusters_in_netlist(void) const; + const std::vector& get_all_traffic_flow_id(void) const; + // setters /** diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index acbf8d6fd40..2de1ded08f0 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -21,17 +21,14 @@ void initial_noc_placement(void) { /* We need all the traffic flow ids to be able to access them. The range * of traffic flow ids go from 0 to the total number of traffic flows within - * the NoC. So get the upper range here.*/ - int number_of_traffic_flows = noc_traffic_flows_storage->get_number_of_traffic_flows(); - - // go through all the traffic flows and route them. Then once routed, update the links used in the routed traffic flows with their usages - for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { - // get the traffic flow with the current id - NocTrafficFlowId conv_traffic_flow_id = (NocTrafficFlowId)traffic_flow_id; - const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(conv_traffic_flow_id); + * the NoC. + * go through all the traffic flows and route them. Then once routed, update the links used in the routed traffic flows with their usages + */ + for (const auto& traffic_flow_id : noc_traffic_flows_storage->get_all_traffic_flow_id()) { + const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); // update the traffic flow route based on where the router cluster blocks are placed - std::vector& curr_traffic_flow_route = get_traffic_flow_route(conv_traffic_flow_id, noc_ctx.noc_model, *noc_traffic_flows_storage, *noc_ctx.noc_flows_router, place_ctx.block_locs); + std::vector& curr_traffic_flow_route = get_traffic_flow_route(traffic_flow_id, noc_ctx.noc_model, *noc_traffic_flows_storage, *noc_ctx.noc_flows_router, place_ctx.block_locs); // update the links used in the found traffic flow route, links' bandwidth should be incremented since the traffic flow is routed update_traffic_flow_link_usage(curr_traffic_flow_route, noc_ctx.noc_model, 1, curr_traffic_flow.traffic_flow_bandwidth); @@ -219,16 +216,14 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no } void recompute_noc_costs(double& new_noc_aggregate_bandwidth_cost, double& new_noc_latency_cost) { - int number_of_traffic_flows = g_vpr_ctx.noc().noc_traffic_flows_storage.get_number_of_traffic_flows(); - // reset the cost variables first new_noc_aggregate_bandwidth_cost = 0; new_noc_latency_cost = 0; // go through the costs of all the traffic flows and add them up to recompute the total costs associated with the NoC - for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { - new_noc_aggregate_bandwidth_cost += traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].aggregate_bandwidth; - new_noc_latency_cost += traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].latency; + for (const auto& traffic_flow_id : g_vpr_ctx.noc().noc_traffic_flows_storage.get_all_traffic_flow_id()) { + new_noc_aggregate_bandwidth_cost += traffic_flow_costs[traffic_flow_id].aggregate_bandwidth; + new_noc_latency_cost += traffic_flow_costs[traffic_flow_id].latency; } return; @@ -248,20 +243,18 @@ double comp_noc_aggregate_bandwidth_cost(void) { // datastructure that stores all the traffic flow routes const NocTrafficFlows* noc_traffic_flows_storage = &noc_ctx.noc_traffic_flows_storage; - int number_of_traffic_flows = noc_traffic_flows_storage->get_number_of_traffic_flows(); - double noc_aggregate_bandwidth_cost = 0.; // now go through each traffic flow route and calculate its // aggregate bandwidth. Then store this in local data structures and accumulate it. - for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { - const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); - const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id); + for (const auto& traffic_flow_id : g_vpr_ctx.noc().noc_traffic_flows_storage.get_all_traffic_flow_id()){ + const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); + const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route(traffic_flow_id); double curr_traffic_flow_aggregate_bandwidth_cost = calculate_traffic_flow_aggregate_bandwidth_cost(curr_traffic_flow_route, curr_traffic_flow); // store the calculated aggregate bandwidth for the current traffic flow in local datastructures (this also initializes them) - traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].aggregate_bandwidth = curr_traffic_flow_aggregate_bandwidth_cost; + traffic_flow_costs[traffic_flow_id].aggregate_bandwidth = curr_traffic_flow_aggregate_bandwidth_cost; // accumulate the aggregate bandwidth cost noc_aggregate_bandwidth_cost += curr_traffic_flow_aggregate_bandwidth_cost; @@ -282,14 +275,14 @@ double comp_noc_latency_cost(const t_noc_opts& noc_opts) { // now go through each traffic flow route and calculate its // latency. Then store this in local data structures and accumulate it. - for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { - const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); - const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id); + for (const auto& traffic_flow_id : g_vpr_ctx.noc().noc_traffic_flows_storage.get_all_traffic_flow_id()){ + const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); + const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route(traffic_flow_id); double curr_traffic_flow_latency_cost = calculate_traffic_flow_latency_cost(curr_traffic_flow_route, noc_ctx.noc_model, curr_traffic_flow, noc_opts); // store the calculated latency for the current traffic flow in local datastructures (this also initializes them) - traffic_flow_costs[(NocTrafficFlowId)traffic_flow_id].latency = curr_traffic_flow_latency_cost; + traffic_flow_costs[traffic_flow_id].latency = curr_traffic_flow_latency_cost; // accumulate the aggregate bandwidth cost noc_latency_cost += curr_traffic_flow_latency_cost; @@ -321,9 +314,9 @@ int check_noc_placement_costs(const t_placer_costs& costs, double error_toleranc std::vector temp_found_noc_route; // go through all the traffic flows and find a route for them based on where the routers are placed within the NoC - for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { + for(const auto& traffic_flow_id : noc_traffic_flows_storage->get_all_traffic_flow_id()){ // get the traffic flow with the current id - const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); + const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); // get the source and destination logical router blocks in the current traffic flow ClusterBlockId logical_source_router_block_id = curr_traffic_flow.source_router_cluster_id; @@ -409,9 +402,9 @@ int get_number_of_traffic_flows_with_latency_cons_met(void) { int count_of_achieved_latency_cons = 0; // now go through each traffic flow route and check if its latency constraint was met - for (int traffic_flow_id = 0; traffic_flow_id < number_of_traffic_flows; traffic_flow_id++) { - const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_id); - const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route((NocTrafficFlowId)traffic_flow_id); + for (const auto& traffic_flow_id : noc_traffic_flows_storage->get_all_traffic_flow_id()) { + const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); + const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route(traffic_flow_id); // there will always be one more router than links in a traffic flow int num_of_links_in_traffic_flow = curr_traffic_flow_route.size(); From a46d0818977536b0df902fba58d970715cda3c7b Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 12:15:29 -0400 Subject: [PATCH 18/29] changed NoC data structures and function call to avoid passing unnecessary function arguments --- vpr/src/noc/noc_traffic_flows.h | 8 ++++++++ vpr/src/place/noc_place_utils.cpp | 31 +++++++++++-------------------- vpr/src/place/noc_place_utils.h | 11 ++--------- vpr/src/place/place.cpp | 5 ++--- 4 files changed, 23 insertions(+), 32 deletions(-) diff --git a/vpr/src/noc/noc_traffic_flows.h b/vpr/src/noc/noc_traffic_flows.h index e3891ddf775..80ee2b37339 100644 --- a/vpr/src/noc/noc_traffic_flows.h +++ b/vpr/src/noc/noc_traffic_flows.h @@ -212,8 +212,16 @@ class NocTrafficFlows { */ std::vector& get_mutable_traffic_flow_route(NocTrafficFlowId traffic_flow_id); + /** + * @return provides access to a vector containing all logical router ClusterBlockId to + * help "propose_router_swap" function to choose a random logical router to move. + */ const std::vector& get_router_clusters_in_netlist(void) const; + /** + * @return provides access to all traffic flows' ids to allow a range-based + * loop through all traffic flows, used in noc_place_utils.cpp functions. + */ const std::vector& get_all_traffic_flow_id(void) const; // setters diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 2de1ded08f0..28a30f66fd0 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -10,7 +10,7 @@ static std::vector affected_traffic_flows; /*********************************************************** *****************************/ void initial_noc_placement(void) { - // need to get placement information about where the router cluster blocks are palced on the device + // need to get placement information about where the router cluster blocks are placed on the device const auto& place_ctx = g_vpr_ctx.placement(); // need to update the link usages within after routing all the traffic flows @@ -37,7 +37,7 @@ void initial_noc_placement(void) { return; } -int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved& blocks_affected, double& noc_aggregate_bandwidth_delta_c, double& noc_latency_delta_c, const t_noc_opts& noc_opts) { +void find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved& blocks_affected, double& noc_aggregate_bandwidth_delta_c, double& noc_latency_delta_c, const t_noc_opts& noc_opts) { // provides the positions where the affected blocks have moved to auto& place_ctx = g_vpr_ctx.placement(); auto& noc_ctx = g_vpr_ctx.mutable_noc(); @@ -48,7 +48,6 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved // This is useful for cases where two moved routers were part of the same traffic flow and prevents us from re-routing the same flow twice. std::unordered_set updated_traffic_flows; - int number_of_affected_traffic_flows = 0; affected_traffic_flows.clear(); // go through the moved blocks and process them only if they are NoC routers @@ -58,7 +57,7 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved // check if the current moved block is a noc router if (noc_traffic_flows_storage->check_if_cluster_block_has_traffic_flows(blk)) { // current block is a router, so re-route all the traffic flows it is a part of - re_route_associated_traffic_flows(blk, *noc_traffic_flows_storage, noc_ctx.noc_model, *noc_ctx.noc_flows_router, place_ctx.block_locs, updated_traffic_flows, number_of_affected_traffic_flows); + re_route_associated_traffic_flows(blk, *noc_traffic_flows_storage, noc_ctx.noc_model, *noc_ctx.noc_flows_router, place_ctx.block_locs, updated_traffic_flows); } } @@ -77,7 +76,6 @@ int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved noc_latency_delta_c += proposed_traffic_flow_costs[traffic_flow_id].latency - traffic_flow_costs[traffic_flow_id].latency; } - return number_of_affected_traffic_flows; } void commit_noc_costs() { @@ -115,7 +113,7 @@ std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, int how_to_update_links, double traffic_flow_bandwidth) { // go through the links within the traffic flow route and update their bandwidth usage for (auto& link_in_route_id : traffic_flow_route) { - // get the link to update and its current bandwdith + // get the link to update and its current bandwidth NocLink& curr_link = noc_model.get_single_mutable_noc_link(link_in_route_id); double curr_link_bandwidth = curr_link.get_bandwidth_usage(); @@ -128,7 +126,7 @@ void update_traffic_flow_link_usage(const std::vector& traffic_flow_r return; } -void re_route_associated_traffic_flows(ClusterBlockId moved_block_router_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations, std::unordered_set& updated_traffic_flows, int& number_of_affected_traffic_flows) { +void re_route_associated_traffic_flows(ClusterBlockId moved_block_router_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations, std::unordered_set& updated_traffic_flows) { // get all the associated traffic flows for the logical router cluster block const std::vector* assoc_traffic_flows = noc_traffic_flows_storage.get_traffic_flows_associated_to_router_block(moved_block_router_id); @@ -146,7 +144,6 @@ void re_route_associated_traffic_flows(ClusterBlockId moved_block_router_id, Noc // update global datastructures to indicate that the current traffic flow was affected due to router cluster blocks being swapped affected_traffic_flows.push_back(traffic_flow_id); - number_of_affected_traffic_flows++; } } } @@ -177,7 +174,7 @@ void revert_noc_traffic_flow_routes(const t_pl_blocks_to_be_moved& blocks_affect const std::vector* assoc_traffic_flows = noc_traffic_flows_storage->get_traffic_flows_associated_to_router_block(blk); // now check if there are any associated traffic flows - if (assoc_traffic_flows != nullptr) { + if (assoc_traffic_flows->size() != 0) { // There are traffic flows associated to the current router block so process them for (auto& traffic_flow_id : *assoc_traffic_flows) { // first check to see whether we have already reverted the current traffic flow and only revert it if we haven't already. @@ -269,13 +266,11 @@ double comp_noc_latency_cost(const t_noc_opts& noc_opts) { // datastructure that stores all the traffic flow routes const NocTrafficFlows* noc_traffic_flows_storage = &noc_ctx.noc_traffic_flows_storage; - int number_of_traffic_flows = noc_traffic_flows_storage->get_number_of_traffic_flows(); - double noc_latency_cost = 0.; // now go through each traffic flow route and calculate its // latency. Then store this in local data structures and accumulate it. - for (const auto& traffic_flow_id : g_vpr_ctx.noc().noc_traffic_flows_storage.get_all_traffic_flow_id()){ + for (const auto& traffic_flow_id : noc_ctx.noc_traffic_flows_storage.get_all_traffic_flow_id()){ const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route(traffic_flow_id); @@ -308,8 +303,6 @@ int check_noc_placement_costs(const t_placer_costs& costs, double error_toleranc NocRoutingAlgorithmCreator routing_algorithm_factory; NocRouting* temp_noc_routing_algorithm = routing_algorithm_factory.create_routing_algorithm(noc_opts.noc_routing_algorithm); - int number_of_traffic_flows = noc_traffic_flows_storage->get_number_of_traffic_flows(); - // stores a temporarily found route for a traffic flow std::vector temp_found_noc_route; @@ -348,7 +341,7 @@ int check_noc_placement_costs(const t_placer_costs& costs, double error_toleranc error++; } - // only check the recomputed cost if it is above our expected latency cost threshold of 1 picosecond, otherwise there is no point in checking it + // only check the recomputed cost if it is above our expected latency cost threshold of 1 pico-second, otherwise there is no point in checking it if (noc_latency_cost_check > MIN_EXPECTED_NOC_LATENCY_COST) { // check whether the latency placement cost is within the error tolerance if (fabs(noc_latency_cost_check - costs.noc_latency_cost) > costs.noc_latency_cost * error_tolerance) { @@ -367,7 +360,7 @@ int check_noc_placement_costs(const t_placer_costs& costs, double error_toleranc double calculate_traffic_flow_aggregate_bandwidth_cost(const std::vector& traffic_flow_route, const t_noc_traffic_flow& traffic_flow_info) { int num_of_links_in_traffic_flow = traffic_flow_route.size(); - // the traffic flow aggregate bandwidth cost is scaled by its priority, which dictates its important to the placement + // the traffic flow aggregate bandwidth cost is scaled by its priority, which dictates its importance to the placement return (traffic_flow_info.traffic_flow_priority * traffic_flow_info.traffic_flow_bandwidth * num_of_links_in_traffic_flow); } @@ -397,8 +390,6 @@ int get_number_of_traffic_flows_with_latency_cons_met(void) { // datastructure that stores all the traffic flow routes const NocTrafficFlows* noc_traffic_flows_storage = &noc_ctx.noc_traffic_flows_storage; - int number_of_traffic_flows = noc_traffic_flows_storage->get_number_of_traffic_flows(); - int count_of_achieved_latency_cons = 0; // now go through each traffic flow route and check if its latency constraint was met @@ -523,7 +514,7 @@ void write_noc_placement_file(std::string file_name) { file_name.c_str()); } - // assume that the FPGA device has a single layer (2-D), so when we write the palcement file the layer value will be constant + // assume that the FPGA device has a single layer (2-D), so when we write the placement file the layer value will be constant int layer_number = 0; // get a reference to the collection of router cluster blocks in the design @@ -548,7 +539,7 @@ void write_noc_placement_file(std::string file_name) { // get the name of the router cluster block const std::string& cluster_name = cluster_block_netlist.block_name(single_cluster_id); - //get the placement location of the curren router cluster block + //get the placement location of the current router cluster block const t_block_loc& cluster_location = clustered_block_placed_locations[single_cluster_id]; // now get the corresponding physical router block id the cluster is located on diff --git a/vpr/src/place/noc_place_utils.h b/vpr/src/place/noc_place_utils.h index d3474e176b4..891889cc420 100644 --- a/vpr/src/place/noc_place_utils.h +++ b/vpr/src/place/noc_place_utils.h @@ -83,11 +83,8 @@ void initial_noc_placement(void); * @param noc_latency_delta_c The change in the overall * NoC latency cost caused by a placer move is stored * here. - * @return An integer which represents the number of traffic flows - * which were affected (modified) due to either their source or - * destination routers being moved for a single placement iteration. */ -int find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved& blocks_affected, double& noc_aggregate_bandwidth_delta_c, double& noc_latency_delta_c, const t_noc_opts& noc_opts); +void find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_moved& blocks_affected, double& noc_aggregate_bandwidth_delta_c, double& noc_latency_delta_c, const t_noc_opts& noc_opts); /** * @brief Updates static datastructures found in 'noc_place_utils.cpp' @@ -179,12 +176,8 @@ void update_traffic_flow_link_usage(const std::vector& traffic_flow_r * placed grid locations of all cluster blocks. * @param updated_traffic_flows Keeps track of traffic flows that have been * re-routed. Used to prevent re-routing the same traffic flow multiple times. - * @param number_of_affected_traffic_flows an integer which represents the - * number of traffic flows which were affected (modified) due to either - * their source or destination routers being moved for a single placement - * iteration. This is updated within this function. */ -void re_route_associated_traffic_flows(ClusterBlockId moved_router_block_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations, std::unordered_set& updated_traffic_flows, int& number_of_affected_traffic_flows); +void re_route_associated_traffic_flows(ClusterBlockId moved_router_block_id, NocTrafficFlows& noc_traffic_flows_storage, NocStorage& noc_model, NocRouting& noc_flows_router, const vtr::vector_map& placed_cluster_block_locations, std::unordered_set& updated_traffic_flows); /** * @brief Used to re-route all the traffic flows associated to logical diff --git a/vpr/src/place/place.cpp b/vpr/src/place/place.cpp index a55f2b51a30..f4a6bf52dfb 100644 --- a/vpr/src/place/place.cpp +++ b/vpr/src/place/place.cpp @@ -1554,14 +1554,13 @@ static e_move_result try_swap(const t_annealing_state* state, delta_c = bb_delta_c * costs->bb_cost_norm; } - int number_of_affected_noc_traffic_flows = 0; double noc_aggregate_bandwidth_delta_c = 0; // change in the NoC aggregate bandwidth cost double noc_latency_delta_c = 0; // change in the NoC latency cost /* Update the NoC datastructure and costs*/ if (noc_opts.noc) { - number_of_affected_noc_traffic_flows = find_affected_noc_routers_and_update_noc_costs(blocks_affected, noc_aggregate_bandwidth_delta_c, noc_latency_delta_c, noc_opts); + find_affected_noc_routers_and_update_noc_costs(blocks_affected, noc_aggregate_bandwidth_delta_c, noc_latency_delta_c, noc_opts); - // Include the NoC delata costs in the total cost change for this swap + // Include the NoC delta costs in the total cost change for this swap delta_c = delta_c + noc_placement_weighting * (noc_latency_delta_c * costs->noc_latency_cost_norm + noc_aggregate_bandwidth_delta_c * costs->noc_aggregate_bandwidth_cost_norm); } From b38dd66c031e3041d3b917769c35c3a9716d9e8a Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 13:29:47 -0400 Subject: [PATCH 19/29] fixed compiler issues with noc unit tests --- vpr/test/test_noc_place_utils.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/vpr/test/test_noc_place_utils.cpp b/vpr/test/test_noc_place_utils.cpp index 3ab03f92fd7..db2e2ab0e95 100644 --- a/vpr/test/test_noc_place_utils.cpp +++ b/vpr/test/test_noc_place_utils.cpp @@ -746,14 +746,14 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ double delta_laten_cost = 0.; // call the test function - int number_of_affected_traffic_flows = find_affected_noc_routers_and_update_noc_costs(blocks_affected, delta_aggr_band_cost, delta_laten_cost, noc_opts); + find_affected_noc_routers_and_update_noc_costs(blocks_affected, delta_aggr_band_cost, delta_laten_cost, noc_opts); // update the test total noc bandwidth and latency costs based on the cost changes found by the test functions test_noc_bandwidth_costs += delta_aggr_band_cost; test_noc_latency_costs += delta_laten_cost; // need this function to update the local datastructures that store all the traffic flow costs - commit_noc_costs(number_of_affected_traffic_flows); + commit_noc_costs(); // clear the affected blocks clear_move_blocks(blocks_affected); @@ -865,14 +865,14 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ double delta_laten_cost = 0.; // call the test function - int number_of_affected_traffic_flows = find_affected_noc_routers_and_update_noc_costs(blocks_affected, delta_aggr_band_cost, delta_laten_cost, noc_opts); + find_affected_noc_routers_and_update_noc_costs(blocks_affected, delta_aggr_band_cost, delta_laten_cost, noc_opts); // update the test total noc bandwidth and latency costs based on the cost changes found by the test functions test_noc_bandwidth_costs += delta_aggr_band_cost; test_noc_latency_costs += delta_laten_cost; // need this function to update the local datastructures that store all the traffic flow costs - commit_noc_costs(number_of_affected_traffic_flows); + commit_noc_costs(); // clear the affected blocks clear_move_blocks(blocks_affected); @@ -945,14 +945,14 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ delta_laten_cost = 0.; // call the test function - number_of_affected_traffic_flows = find_affected_noc_routers_and_update_noc_costs(blocks_affected, delta_aggr_band_cost, delta_laten_cost, noc_opts); + find_affected_noc_routers_and_update_noc_costs(blocks_affected, delta_aggr_band_cost, delta_laten_cost, noc_opts); // update the test total noc bandwidth and latency costs based on the cost changes found by the test functions test_noc_bandwidth_costs += delta_aggr_band_cost; test_noc_latency_costs += delta_laten_cost; // need this function to update the local datastructures that store all the traffic flow costs - commit_noc_costs(number_of_affected_traffic_flows); + commit_noc_costs(); // clear the affected blocks clear_move_blocks(blocks_affected); @@ -997,14 +997,14 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ delta_laten_cost = 0.; // call the test function - number_of_affected_traffic_flows = find_affected_noc_routers_and_update_noc_costs(blocks_affected, delta_aggr_band_cost, delta_laten_cost, noc_opts); + find_affected_noc_routers_and_update_noc_costs(blocks_affected, delta_aggr_band_cost, delta_laten_cost, noc_opts); // update the test total noc bandwidth and latency costs based on the cost changes found by the test functions test_noc_bandwidth_costs += delta_aggr_band_cost; test_noc_latency_costs += delta_laten_cost; // need this function to update the local datastructures that store all the traffic flow costs - commit_noc_costs(number_of_affected_traffic_flows); + commit_noc_costs(); // clear the affected blocks clear_move_blocks(blocks_affected); From ed122fd040d70c183ad5b11236135541a049322b Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 13:32:08 -0400 Subject: [PATCH 20/29] make format --- vpr/src/noc/noc_traffic_flows.cpp | 2 +- vpr/src/place/noc_place_utils.cpp | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index 3afa8d39d14..d28859c48ad 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -48,7 +48,7 @@ const std::vector& NocTrafficFlows::get_router_clusters_in_netli return router_cluster_in_netlist; } -const std::vector& NocTrafficFlows::get_all_traffic_flow_id(void) const{ +const std::vector& NocTrafficFlows::get_all_traffic_flow_id(void) const { return noc_traffic_flows_ids; } diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 28a30f66fd0..3b374ea8333 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -75,11 +75,10 @@ void find_affected_noc_routers_and_update_noc_costs(const t_pl_blocks_to_be_move noc_aggregate_bandwidth_delta_c += proposed_traffic_flow_costs[traffic_flow_id].aggregate_bandwidth - traffic_flow_costs[traffic_flow_id].aggregate_bandwidth; noc_latency_delta_c += proposed_traffic_flow_costs[traffic_flow_id].latency - traffic_flow_costs[traffic_flow_id].latency; } - } void commit_noc_costs() { - for (auto& traffic_flow_id : affected_traffic_flows){ + for (auto& traffic_flow_id : affected_traffic_flows) { // update the traffic flow costs traffic_flow_costs[traffic_flow_id] = proposed_traffic_flow_costs[traffic_flow_id]; @@ -244,7 +243,7 @@ double comp_noc_aggregate_bandwidth_cost(void) { // now go through each traffic flow route and calculate its // aggregate bandwidth. Then store this in local data structures and accumulate it. - for (const auto& traffic_flow_id : g_vpr_ctx.noc().noc_traffic_flows_storage.get_all_traffic_flow_id()){ + for (const auto& traffic_flow_id : g_vpr_ctx.noc().noc_traffic_flows_storage.get_all_traffic_flow_id()) { const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route(traffic_flow_id); @@ -270,7 +269,7 @@ double comp_noc_latency_cost(const t_noc_opts& noc_opts) { // now go through each traffic flow route and calculate its // latency. Then store this in local data structures and accumulate it. - for (const auto& traffic_flow_id : noc_ctx.noc_traffic_flows_storage.get_all_traffic_flow_id()){ + for (const auto& traffic_flow_id : noc_ctx.noc_traffic_flows_storage.get_all_traffic_flow_id()) { const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); const std::vector& curr_traffic_flow_route = noc_traffic_flows_storage->get_traffic_flow_route(traffic_flow_id); @@ -307,7 +306,7 @@ int check_noc_placement_costs(const t_placer_costs& costs, double error_toleranc std::vector temp_found_noc_route; // go through all the traffic flows and find a route for them based on where the routers are placed within the NoC - for(const auto& traffic_flow_id : noc_traffic_flows_storage->get_all_traffic_flow_id()){ + for (const auto& traffic_flow_id : noc_traffic_flows_storage->get_all_traffic_flow_id()) { // get the traffic flow with the current id const t_noc_traffic_flow& curr_traffic_flow = noc_traffic_flows_storage->get_single_noc_traffic_flow(traffic_flow_id); From 36beac20436f62e418f4b7267a3263475382599e Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 15:22:57 -0400 Subject: [PATCH 21/29] fixed typos in NoC-related code --- vpr/src/noc/noc_link.h | 2 +- vpr/src/noc/noc_router.h | 2 +- vpr/src/noc/noc_routing.h | 2 +- vpr/src/noc/noc_storage.cpp | 12 +++--- vpr/src/noc/noc_storage.h | 42 +++++++++---------- vpr/src/noc/noc_traffic_flows.cpp | 10 ++--- vpr/src/noc/noc_traffic_flows.h | 8 ++-- .../noc/read_xml_noc_traffic_flows_file.cpp | 8 ++-- vpr/src/noc/read_xml_noc_traffic_flows_file.h | 16 +++---- vpr/src/noc/xy_routing.cpp | 6 +-- vpr/src/noc/xy_routing.h | 8 ++-- vpr/src/place/noc_place_utils.h | 16 +++---- vpr/test/test_noc_place_utils.cpp | 10 ++--- 13 files changed, 71 insertions(+), 71 deletions(-) diff --git a/vpr/src/noc/noc_link.h b/vpr/src/noc/noc_link.h index b1b60d0858c..e35802a2f2a 100644 --- a/vpr/src/noc/noc_link.h +++ b/vpr/src/noc/noc_link.h @@ -84,7 +84,7 @@ class NocLink { /** * @brief Can be used to set the sink router of the link to a different router. - * @param source_router The identifier representing the router that should be the sibk of + * @param sink_router The identifier representing the router that should be the sink of * this link */ void set_sink_router(NocRouterId sink); diff --git a/vpr/src/noc/noc_router.h b/vpr/src/noc/noc_router.h index 9568ba6740d..337dabc7921 100644 --- a/vpr/src/noc/noc_router.h +++ b/vpr/src/noc/noc_router.h @@ -39,7 +39,7 @@ class NocRouter { private: /** this represents a unique id provided by the user when describing the NoC topology in the arch file. The intended - * use is to report errors with rouer ids the user understands*/ + * use is to report errors with router ids the user understands*/ int router_user_id; // device position of the physical router tile diff --git a/vpr/src/noc/noc_routing.h b/vpr/src/noc/noc_routing.h index 29a7ba30bd1..2feb5f513f8 100644 --- a/vpr/src/noc/noc_routing.h +++ b/vpr/src/noc/noc_routing.h @@ -47,7 +47,7 @@ class NocRouting { * @param sink_router_id The destination router of a traffic flow. * Identifies the ending point of the route within the NoC.This represents a * physical router on the FPGA. - * @param flow_route Stores the path returned by this fuction + * @param flow_route Stores the path returned by this function * as a series of NoC links found by * a NoC routing algorithm between two routers in a traffic flow. * The function will clear any diff --git a/vpr/src/noc/noc_storage.cpp b/vpr/src/noc/noc_storage.cpp index 1703d4dac5d..cb5382cd13e 100644 --- a/vpr/src/noc/noc_storage.cpp +++ b/vpr/src/noc/noc_storage.cpp @@ -70,15 +70,15 @@ NocRouterId NocStorage::get_router_at_grid_location(const t_pl_loc& hard_router_ // setters for the NoC -void NocStorage::add_router(int id, int grid_position_x, int grid_posistion_y) { +void NocStorage::add_router(int id, int grid_position_x, int grid_position_y) { VTR_ASSERT_MSG(!built_noc, "NoC already built, cannot modify further."); - router_storage.emplace_back(id, grid_position_x, grid_posistion_y); + router_storage.emplace_back(id, grid_position_x, grid_position_y); /* Get the corresponding NocRouterId for the newly added router and * add it to the conversion table. * Since the router is added at the end of the list, the id is equivalent to the last element index. - * We build the conversion table here as it gurantees only unique routers + * We build the conversion table here as it guarantees only unique routers * in the NoC are added. */ NocRouterId converted_id((int)(router_storage.size() - 1)); @@ -86,7 +86,7 @@ void NocStorage::add_router(int id, int grid_position_x, int grid_posistion_y) { /* need to associate the current router with its grid position */ // get the key to identify the current router - int router_key = generate_router_key_from_grid_location(grid_position_x, grid_posistion_y); + int router_key = generate_router_key_from_grid_location(grid_position_x, grid_position_y); grid_location_to_router_id.insert(std::pair(router_key, converted_id)); return; @@ -127,12 +127,12 @@ bool NocStorage::remove_link(NocRouterId src_router_id, NocRouterId sink_router_ // This status variable is used to report externally whether the link was removed or not bool link_removed_status = false; - // check if the src router for the link to remove exists (within the id ranges). Otherwise there is no point looking for the link + // check if the src router for the link to remove exists (within the id ranges). Otherwise, there is no point looking for the link if ((size_t)src_router_id < router_storage.size()) { // get all the outgoing links of the provided sourcer router std::vector* source_router_outgoing_links = &router_link_list[src_router_id]; - // keeps track of the position of each outgoing link for the provided src router. When the id of the link to remove is found, this index can be used to remove it from the ougoing link vector. + // keeps track of the position of each outgoing link for the provided src router. When the id of the link to remove is found, this index can be used to remove it from the outgoing link vector. int outgoing_link_index = 0; // go through each outgoing link of the source router and see if there is a link that also has the corresponding sink router. diff --git a/vpr/src/noc/noc_storage.h b/vpr/src/noc/noc_storage.h index 674bb32bf16..c1d1e025af0 100644 --- a/vpr/src/noc/noc_storage.h +++ b/vpr/src/noc/noc_storage.h @@ -27,7 +27,7 @@ * A link is a component of the NoC ans is defined by the * NocLink class. Links are connections between two routers. * Links are used by routers to communicate with other routers - * in the NoC. Thye can be thought of as edges in a graph. Links + * in the NoC. They can be thought of as edges in a graph. Links * have a source router where they exit from and sink router where * they enter. It is important to note that the links are not * unidirectional, the legal way to traverse a link is from the @@ -69,10 +69,10 @@ class NocStorage { * @brief The user provides an ID for the router when describing the NoC * in the architecture file. This ID system will be different than the * NocRouterIds assigned to each router. The user ID system will be - * arbritary but the internal ID system used here will start at 0 and + * arbitrary but the internal ID system used here will start at 0 and * are dense since it is used to index the routers. The datastructure * below is a conversiont able that maps the user router IDs to the - * correspondint internal ones. + * corresponding internal ones. * */ std::unordered_map router_id_conversion_table; @@ -84,9 +84,9 @@ class NocStorage { * logical router was moved is known. * Using this datastructure, the grid location can be used to * identify the corresponding hard router block positioned at that grid - * location. The NocROuterId uniqely identifies hard router blocks and + * location. The NocROuterId uniquely identifies hard router blocks and * can be used to retrieve the hard router block information using - * the router_storage datastructurre above. This can also be used to + * the router_storage data structure above. This can also be used to * access the connectivity graph datastructure above. * * It is important to know the specific hard router block because @@ -96,7 +96,7 @@ class NocStorage { * the placement cost of the moved logical router block. * * The intended use is when trying to re-route a traffic flow. The current - * location of a logical router block can be used in conjuction with this + * location of a logical router block can be used in conjunction with this * datastructure to identify the corresponding hard router block. * */ @@ -105,7 +105,7 @@ class NocStorage { /** * @brief A flag that indicates whether the NoC has been built. If this * flag is true, then the NoC cannot be modified, meaning that routers and - * links cannot be added or removed. The inteded use of this flag is to + * links cannot be added or removed. The intended use of this flag is to * set it after you complete building the NoC (adding routers and links). * This flag can then acts as a check so that the NoC is not modified * later on after building it. @@ -144,7 +144,7 @@ class NocStorage { void operator=(const NocStorage&) = delete; public: - // default contructor (cleare all the elements in the vectors) + // default constructor (clear all the elements in the vectors) NocStorage(); // getters for the NoC @@ -153,7 +153,7 @@ class NocStorage { * @brief Gets a vector of outgoing links for a given router * in the NoC. THe link vector cannot be modified. * - * @param id A unique indetifier that represents a router + * @param id A unique identifier that represents a router * @return A vector of links. The links are represented by a unique * identifier. */ @@ -193,7 +193,7 @@ class NocStorage { * @brief Get the maximum allowable bandwidth for a link * within the NoC. * - * @return a numeric value that represents the link bandwith in bps + * @return a numeric value that represents the link bandwidth in bps */ double get_noc_link_bandwidth(void) const; @@ -266,7 +266,7 @@ class NocStorage { * router block positioned on that grid location. * * @param hard_router_location A struct that contains the grid location - * of an arbirtary hard router block on the FPGA. + * of an arbitrary hard router block on the FPGA. * @return NocRouterId The hard router block "id" * located at the given grid location. */ @@ -283,9 +283,9 @@ class NocStorage { * the NoC. * * @param id The user supplied identification for the router. - * @param grid_position_x The horizontal position on the FPGA of the phyical + * @param grid_position_x The horizontal position on the FPGA of the physical * tile that this router represents. - * @param grid_position_y The vertical position on the FPGA of the phyical + * @param grid_position_y The vertical position on the FPGA of the physical * tile that this router represents. */ void add_router(int id, int grid_position_x, int grid_position_y); @@ -336,7 +336,7 @@ class NocStorage { void set_device_grid_width(int grid_width); - // general utiliy functions + // general utility functions /** * @brief The link is removed from the outgoing vector of links for * the source router. The link is not removed from the vector of all @@ -344,7 +344,7 @@ class NocStorage { * the link is set to being invalid by. The link * is still removed since it will be considered invalid when used * externally. THe link is identified by going through the vector - * outgoing links of the supplied source router, for each outgoin link + * outgoing links of the supplied source router, for each outgoing link * the sink router is compared the supplied sink router and the link to * remove is identified if there is a match. * If the link doesn't exist in the @@ -353,7 +353,7 @@ class NocStorage { * * @param src_router_id The source router of the traffic flow to delete * @param sink_router_id The sink router of the traffic flow to delete - * @return true The link was succesfully removed + * @return true The link was successfully removed * @return false The link was not removed */ bool remove_link(NocRouterId src_router_id, NocRouterId sink_router_id); @@ -369,8 +369,8 @@ class NocStorage { void finished_building_noc(void); /** - * @brief Resets the NoC by clearning all internal datastructures. - * This includes deleteing all routers and links. Also all internal + * @brief Resets the NoC by clearing all internal datastructures. + * This includes deleting all routers and links. Also all internal * IDs are removed (the is conversion table is cleared). It is * recommended to run this function before building the NoC. * @@ -431,9 +431,9 @@ class NocStorage { * The key will be generated as follows: * key = y * device_grid.width() + x * - * @param grid_position_x The horizontal position on the FPGA of the phyical + * @param grid_position_x The horizontal position on the FPGA of the physical * tile that this router represents. - * @param grid_position_y The vertical position on the FPGA of the phyical + * @param grid_position_y The vertical position on the FPGA of the physical * tile that this router represents. * @return int Represents a unique key that can be used to identify a * hard router block. @@ -441,7 +441,7 @@ class NocStorage { int generate_router_key_from_grid_location(int grid_position_x, int grid_position_y) const; /** - * @brief Writes out the NocStirage class infromation to a file. + * @brief Writes out the NocStorage class information to a file. * This includes the list of routers and their connections * to other routers in the NoC. * diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index d28859c48ad..29d12217ab0 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -80,7 +80,7 @@ void NocTrafficFlows::set_router_cluster_in_netlist(std::vector // utility functions for the noc traffic flows -void NocTrafficFlows::finshed_noc_traffic_flows_setup(void) { +void NocTrafficFlows::finished_noc_traffic_flows_setup(void) { // all the traffic flows have been added, so indicate that the class has been constructed and cannot be modified anymore built_traffic_flows = true; @@ -107,7 +107,7 @@ void NocTrafficFlows::clear_traffic_flows(void) { bool NocTrafficFlows::check_if_cluster_block_has_traffic_flows(ClusterBlockId block_id) { auto traffic_flows = get_traffic_flows_associated_to_router_block(block_id); - // indicate whether a vector of traffic flows were found that are associated to the curre cluster block + // indicate whether a vector of traffic flows were found that are associated to the current cluster block return (traffic_flows != nullptr); } @@ -117,7 +117,7 @@ void NocTrafficFlows::add_traffic_flow_to_associated_routers(NocTrafficFlowId tr // get a reference to the traffic flows associated with the current router auto router_traffic_flows = traffic_flows_associated_to_router_blocks.find(associated_router_id); - // check if a vector asssociated traffic flows exists + // check if a vector associated traffic flows exists if (router_traffic_flows == traffic_flows_associated_to_router_blocks.end()) { // there exists no associated traffic flows for this router, so we add it with the newly created traffic flow id traffic_flows_associated_to_router_blocks.insert(std::pair>(associated_router_id, {traffic_flow_id})); @@ -157,7 +157,7 @@ void NocTrafficFlows::echo_noc_traffic_flows(char* file_name) { fprintf(fp, "Traffic flow bandwidth: %f bps\n", traffic_flow->traffic_flow_bandwidth); fprintf(fp, "Traffic flow latency: %f seconds\n", traffic_flow->max_traffic_flow_latency); - // seperate the next link information + // separate the next link information fprintf(fp, "\n"); // update the id for the next traffic flow @@ -184,7 +184,7 @@ void NocTrafficFlows::echo_noc_traffic_flows(char* file_name) { fprintf(fp, "%lu ", (size_t)*traffic_flow); } - // seperate to the next cluster associated traffic flows information + // separate to the next cluster associated traffic flows information fprintf(fp, "\n\n"); } diff --git a/vpr/src/noc/noc_traffic_flows.h b/vpr/src/noc/noc_traffic_flows.h index 80ee2b37339..2ec4edc73f0 100644 --- a/vpr/src/noc/noc_traffic_flows.h +++ b/vpr/src/noc/noc_traffic_flows.h @@ -4,7 +4,7 @@ /** * @file * @brief This file defines the NocTrafficFlows class, which contains all - * communication betwee routers in the NoC. + * communication between routers in the NoC. * * Overview * ======== @@ -60,7 +60,7 @@ struct t_noc_traffic_flow { /** The maximum allowable time to transmit data between thw two routers, in seconds. This parameter will be used to evaluate a router traffic flow.*/ double max_traffic_flow_latency; - /** Indicates the importance of the traffic flow. Higher priority traffic flows will have more importance and will be more likely to have their latency reduced and constriants met. Range: [0-inf) */ + /** Indicates the importance of the traffic flow. Higher priority traffic flows will have more importance and will be more likely to have their latency reduced and constraints met. Range: [0-inf) */ int traffic_flow_priority; /** Constructor initializes all variables*/ @@ -132,7 +132,7 @@ class NocTrafficFlows { * to a vector of traffic flows associated to the router. * * @param traffic_flow_id A unique id that represents a traffic flow. - * @param associated_router_id A ClusterblockId that represents a + * @param associated_router_id A ClusterBlockId that represents a * router block. * @param router_associated_traffic_flows A datastructure that stores * a vector of traffic flows for a given router block where the traffic @@ -280,7 +280,7 @@ class NocTrafficFlows { * */ - void finshed_noc_traffic_flows_setup(void); + void finished_noc_traffic_flows_setup(void); /** * @brief Resets the class by clearing internal diff --git a/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp b/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp index e6139b229ae..7a20109d041 100644 --- a/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp +++ b/vpr/src/noc/read_xml_noc_traffic_flows_file.cpp @@ -30,7 +30,7 @@ void read_xml_noc_traffic_flows_file(const char* noc_flows_file) { */ std::vector cluster_blocks_compatible_with_noc_router_tiles = get_cluster_blocks_compatible_with_noc_router_tiles(cluster_ctx, noc_router_tile_type); - /* variabled used when parsing the file. + /* variable used when parsing the file. * Stores xml related information while parsing the file, such as current * line number, current tag and etc. These variables will be used to * provide additional information to the user when reporting an error. @@ -66,7 +66,7 @@ void read_xml_noc_traffic_flows_file(const char* noc_flows_file) { // make sure that all the router modules in the design have an associated traffic flow check_that_all_router_blocks_have_an_associated_traffic_flow(noc_ctx, noc_router_tile_type, noc_flows_file); - noc_ctx.noc_traffic_flows_storage.finshed_noc_traffic_flows_setup(); + noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // dump out the NocTrafficFlows class information if the user requested it if (getEchoEnabled() && isEchoFileEnabled(E_ECHO_NOC_TRAFFIC_FLOWS)) { @@ -154,7 +154,7 @@ double get_max_traffic_flow_latency(pugi::xml_node single_flow_tag, const pugiut } int get_traffic_flow_priority(pugi::xml_node single_flow_tag, const pugiutil::loc_data& loc_data) { - // default priority is 1 (indicating that the traffic flow is weighted equall to all others) + // default priority is 1 (indicating that the traffic flow is weighted equal to all others) int traffic_flow_priority = 1; // get the corresponding attribute where the priority is stored @@ -248,7 +248,7 @@ t_physical_tile_type_ptr get_physical_type_of_noc_router_tile(const DeviceContex // assuming that all routers have the same physical type, so we are just using the physical type of the first router stored within the NoC auto physical_noc_router = noc_ctx.noc_model.get_noc_routers().begin(); - // Cannot gurantee that there are any physical routers within the NoC, so check if the NoC has any routers, if it doesn't then throw an error + // Cannot guarantee that there are any physical routers within the NoC, so check if the NoC has any routers, if it doesn't then throw an error VTR_ASSERT(physical_noc_router != noc_ctx.noc_model.get_noc_routers().end()); //Using the routers grid position go to the device and identify the physical type of the tile located there. diff --git a/vpr/src/noc/read_xml_noc_traffic_flows_file.h b/vpr/src/noc/read_xml_noc_traffic_flows_file.h index 28649638515..e8005665b3c 100644 --- a/vpr/src/noc/read_xml_noc_traffic_flows_file.h +++ b/vpr/src/noc/read_xml_noc_traffic_flows_file.h @@ -39,12 +39,12 @@ // identifier when an integer conversion failed while reading an attribute value in an xml file constexpr int NUMERICAL_ATTRIBUTE_CONVERSION_FAILURE = -1; -// defines the latency constriant of a traffic flow when not provided by the user -// This value has to be signifigantly larger than latencies seen within the NoC so that the net effect in the placement cost is 0 (the latency constraint has no effect since there is none) -// Since the traffic flow latencies will be in nanoseconds, setting this value to 1 second which is signifigantly larger that what will be seen in the NoC +// defines the latency constraint of a traffic flow when not provided by the user +// This value has to be significantly larger than latencies seen within the NoC so that the net effect in the placement cost is 0 (the latency constraint has no effect since there is none) +// Since the traffic flow latencies will be in nanoseconds, setting this value to 1 second which is significantly larger that what will be seen in the NoC constexpr double DEFAULT_MAX_TRAFFIC_FLOW_LATENCY = 1.; -// defines the prirority of a traffic flow when not specified by a user +// defines the priority of a traffic flow when not specified by a user constexpr int DEFAULT_TRAFFIC_FLOW_PRIORITY = 1; /** @@ -52,7 +52,7 @@ constexpr int DEFAULT_TRAFFIC_FLOW_PRIORITY = 1; * in the NoC. A traffic flow is a communication between one router * in the NoC to another. The XML file contains a number of these traffic * flows and provides additional information about them, such as the - * size of data being tranferred and constraints on the latency of the + * size of data being transferred and constraints on the latency of the * data transmission. Once the traffic flows are parsed, they are stored * inside the NocTrafficFlows class. * @@ -149,7 +149,7 @@ void verify_traffic_flow_router_modules(std::string source_router_name, std::str * priority are all non-negative. An error is thrown if the * above conditions are not met. * - * @param traffic_flow_bandwidth The transmission size betwee the two routers + * @param traffic_flow_bandwidth The transmission size between the two routers * in the traffic flow. * @param max_traffic_flow_latency The allowable latency for the data * transmission between the two routers in the @@ -207,7 +207,7 @@ ClusterBlockId get_router_module_cluster_id(std::string router_module_name, cons void check_traffic_flow_router_module_type(std::string router_module_name, ClusterBlockId router_module_id, pugi::xml_node single_flow_tag, const pugiutil::loc_data& loc_data, const ClusteringContext& cluster_ctx, t_physical_tile_type_ptr noc_router_tile_type); /** - * @brief Retreives the physical type of a noc router tile. + * @brief Retrieves the physical type of a noc router tile. * * @param device_ctx Contains the device information. Has a datastructure that * can determine a tile type based on grid position on the @@ -240,7 +240,7 @@ t_physical_tile_type_ptr get_physical_type_of_noc_router_tile(const DeviceContex bool check_that_all_router_blocks_have_an_associated_traffic_flow(NocContext& noc_ctx, t_physical_tile_type_ptr noc_router_tile_type, std::string noc_flows_file); /** - * @brief Goes through the blocks within the clustered netlist and indetifies + * @brief Goes through the blocks within the clustered netlist and identifies * all blocks that are compatible with a NoC router tile. BY compatible * it means that we can place the cluster block on a NoC router tile. * The run time for this function is O(N) where N is the number of diff --git a/vpr/src/noc/xy_routing.cpp b/vpr/src/noc/xy_routing.cpp index 80d83af7775..6389b234c02 100644 --- a/vpr/src/noc/xy_routing.cpp +++ b/vpr/src/noc/xy_routing.cpp @@ -17,7 +17,7 @@ void XYRouting::route_flow(NocRouterId src_router_id, NocRouterId sink_router_id // keep track of the last router in the route as we build it. Initially we are at the start router, so that will be the current router NocRouterId curr_router_id = src_router_id; - // lets get the sink router + // get the sink router const NocRouter& sink_router = noc_model.get_single_noc_router(sink_router_id); // get the position of the sink router @@ -99,13 +99,13 @@ bool XYRouting::move_to_next_router(NocRouterId& curr_router_id, int curr_router // keeps track of whether a router was found that we can move to bool found_next_router = false; - // When a acceptable link is found, this variable keeps track of whether the next router visited using the link was already visited or not. + // When an acceptable link is found, this variable keeps track of whether the next router visited using the link was already visited or not. bool visited_next_router = false; // get all the outgoing links for the current router const std::vector& router_connections = noc_model.get_noc_router_connections(curr_router_id); - // go through each outgoing link and determine whether the link leads towards the inteded route direction + // go through each outgoing link and determine whether the link leads towards the intended route direction for (auto connecting_link = router_connections.begin(); connecting_link != router_connections.end(); connecting_link++) { // get the current outgoing link which is being processed const NocLink& curr_outgoing_link = noc_model.get_single_noc_link(*connecting_link); diff --git a/vpr/src/noc/xy_routing.h b/vpr/src/noc/xy_routing.h index 7825c5d3648..259ea18cac8 100644 --- a/vpr/src/noc/xy_routing.h +++ b/vpr/src/noc/xy_routing.h @@ -29,7 +29,7 @@ * has the same X-coordinate as the destination) the algorithm * checks to see whether the y-axis coordinates match between the destination * router and the current router in the path (checking for vertical alignment). - * Similiar to the x-axis movement, the algorithm moves in the Y-axis towards + * Similar to the x-axis movement, the algorithm moves in the Y-axis towards * the destination router. Once again, at each router in the path the algorithm * checks for vertical alignment; if not aligned it then moves in the y-axis * towards the destination router until it is aligned vertically. @@ -74,8 +74,8 @@ * * Usage * ----- - * It is recommmended to use this algorithm when the NoC topology is of type - * Mesh. This algorithm will work for other types of toplogies but the + * It is recommended to use this algorithm when the NoC topology is of type + * Mesh. This algorithm will work for other types of topologies but the * directional nature of the algorithm makes it ideal for mesh topologies. If * the algorithm fails to find a router then an error is thrown; this should * only happen for non-mesh topologies. @@ -113,7 +113,7 @@ class XYRouting : public NocRouting { * @param sink_router_id The destination router of a traffic flow. * Identifies the ending point of the route within the NoC.This represents a * physical router on the FPGA. - * @param flow_route Stores the path returned by this fuction + * @param flow_route Stores the path returned by this function * as a series of NoC links found by * a NoC routing algorithm between two routers in a traffic flow. * The function will clear any diff --git a/vpr/src/place/noc_place_utils.h b/vpr/src/place/noc_place_utils.h index 891889cc420..dde087ad40c 100644 --- a/vpr/src/place/noc_place_utils.h +++ b/vpr/src/place/noc_place_utils.h @@ -14,9 +14,9 @@ #include "fstream" // represent the maximum values of the NoC cost normalization factors // -// we need to handle the case where the agggregate bandwidth is 0, so we set this to some arbritary positive number that is greater than 1.e-9, since that is the range we expect the normalization factor to be (in Gbps) +// we need to handle the case where the aggregate bandwidth is 0, so we set this to some arbitrary positive number that is greater than 1.e-9, since that is the range we expect the normalization factor to be (in Gbps) constexpr double MAX_INV_NOC_AGGREGATE_BANDWIDTH_COST = 1.; -// we expect the latency costs to be in the picosecond range, and we don't expect it to go lower than that. So if the latency costs go below the picosecond range we trim the normalization value to be no higher than 1/ps +// we expect the latency costs to be in the pico-second range, and we don't expect it to go lower than that. So if the latency costs go below the pico-second range we trim the normalization value to be no higher than 1/ps // This should be updated if the delays become lower constexpr double MAX_INV_NOC_LATENCY_COST = 1.e12; @@ -58,7 +58,7 @@ void initial_noc_placement(void); * * For each moved block that is a NoC router, all the traffic flows * that the router is a part of are re-routed. The individual noc placement - * costs (latency and aggregate bandwdith) are also updated to + * costs (latency and aggregate bandwidth) are also updated to * reflect the re-routed traffic flows. This update is done to the * 'proposed_traffic_flow_aggregate_bandwidth_cost' and * 'proposed_traffic_flow_latency_cost' datastructures found in @@ -131,10 +131,10 @@ std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, /** * @brief Updates the bandwidth usages of links found in a routed traffic flow. - * The link bandwidth usages are either incremeneted or decremented by the + * The link bandwidth usages are either incremented or decremented by the * bandwidth of the traffic flow. If the traffic flow route is being deleted, * then the link bandwidth needs to be decremented. If the traffic flow - * route has just been added then the link badnwidth needs to be incremented. + * route has just been added then the link bandwidth needs to be incremented. * This function needs to be called everytime a traffic flow has been newly * routed. * @@ -203,7 +203,7 @@ void revert_noc_traffic_flow_routes(const t_pl_blocks_to_be_moved& blocks_affect * @param traffic_flow_id The traffic flow to re-route. * @param noc_traffic_flows_storage Contains all the traffic flow information * within the NoC. Used to get the current traffic flow information. - * @param noc_model Contains all the links and rotuters within the NoC. Used + * @param noc_model Contains all the links and routers within the NoC. Used * to route traffic flows within the NoC. * @param noc_flows_router The packet routing algorithm used to route traffic * flows within the NoC. @@ -214,7 +214,7 @@ void re_route_traffic_flow(NocTrafficFlowId traffic_flow_id, NocTrafficFlows& no /** * @brief Recompute the NoC costs (aggregate bandwidth and latency) by - * accumulating the inidividual traffic flow costs and then verify + * accumulating the individual traffic flow costs and then verify * whether the result is within an error tolerance of the placements * NoC costs. * @@ -407,7 +407,7 @@ e_create_move propose_router_swap(t_pl_blocks_to_be_moved& blocks_affected, floa /** * @brief Writes out the locations of the router cluster blocks in the * final placement. This file contains only NoC routers and the - * information is written out in a format that is compatible with the RADsim + * information is written out in a format that is compatible with the RADSim * imulator place file. The output of this function is a text file. * * Sample placement file output: diff --git a/vpr/test/test_noc_place_utils.cpp b/vpr/test/test_noc_place_utils.cpp index db2e2ab0e95..189f4886f1b 100644 --- a/vpr/test/test_noc_place_utils.cpp +++ b/vpr/test/test_noc_place_utils.cpp @@ -132,7 +132,7 @@ TEST_CASE("test_initial_noc_placement", "[noc_place_utils]") { break; } } - noc_ctx.noc_traffic_flows_storage.finshed_noc_traffic_flows_setup(); + noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // now go and route all the traffic flows // // start by creating the routing alagorithm @@ -310,7 +310,7 @@ TEST_CASE("test_initial_comp_cost_functions", "[noc_place_utils]") { } } - noc_ctx.noc_traffic_flows_storage.finshed_noc_traffic_flows_setup(); + noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // need to route all the traffic flows so create a datstructure to store them here std::vector golden_traffic_flow_route_sizes; @@ -559,7 +559,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ break; } } - noc_ctx.noc_traffic_flows_storage.finshed_noc_traffic_flows_setup(); + noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // now go and route all the traffic flows // // start by creating the routing alagorithm @@ -1244,7 +1244,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { } } - noc_ctx.noc_traffic_flows_storage.finshed_noc_traffic_flows_setup(); + noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // now go and route all the traffic flows // // start by creating the routing alagorithm @@ -1559,7 +1559,7 @@ TEST_CASE("test_check_noc_placement_costs", "[noc_place_utils]") { } } - noc_ctx.noc_traffic_flows_storage.finshed_noc_traffic_flows_setup(); + noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // now go and route all the traffic flows // // start by creating the routing alagorithm From 4c36ec63ce6c6aa648baf685e4a3db1fe0b53289 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 17:27:46 -0400 Subject: [PATCH 22/29] clear the internal data structure for traffic flows' ids - resolved unit test bug --- vpr/src/noc/noc_traffic_flows.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index 29d12217ab0..b0f132b26d6 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -94,6 +94,7 @@ void NocTrafficFlows::finished_noc_traffic_flows_setup(void) { void NocTrafficFlows::clear_traffic_flows(void) { // delete any information from internal datastructures noc_traffic_flows.clear(); + noc_traffic_flows_ids.clear(); router_cluster_in_netlist.clear(); traffic_flows_associated_to_router_blocks.clear(); traffic_flow_routes.clear(); From 4775ab991eb784a6bfba612fe59496dd8aa5842f Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 17:29:47 -0400 Subject: [PATCH 23/29] fixed comment typos for test_noc_place_utils --- vpr/test/test_noc_place_utils.cpp | 100 +++++++++++++++--------------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/vpr/test/test_noc_place_utils.cpp b/vpr/test/test_noc_place_utils.cpp index 189f4886f1b..65e76fb4dbb 100644 --- a/vpr/test/test_noc_place_utils.cpp +++ b/vpr/test/test_noc_place_utils.cpp @@ -30,7 +30,7 @@ TEST_CASE("test_initial_noc_placement", "[noc_place_utils]") { auto& noc_ctx = g_vpr_ctx.mutable_noc(); auto& place_ctx = g_vpr_ctx.mutable_placement(); - // start by deleting any global datastructures (this is so that we dont have corruption from previous tests) + // start by deleting any global datastructures (this is so that we don't have corruption from previous tests) noc_ctx.noc_model.clear_noc(); noc_ctx.noc_traffic_flows_storage.clear_traffic_flows(); delete noc_ctx.noc_flows_router; @@ -85,7 +85,7 @@ TEST_CASE("test_initial_noc_placement", "[noc_place_utils]") { } } - // now we need to create router cluster blocks and assing them to placed at a router hard block + // now we need to create router cluster blocks and passing them to placed at a router hard block for (int cluster_block_number = 0; cluster_block_number < NUM_OF_LOGICAL_ROUTER_BLOCKS_NOC_PLACE_UTILS_TEST; cluster_block_number++) { // since the indexes for the hard router blocks start from 0, we will just place the router clusters on hard router blocks with the same id // @@ -99,7 +99,7 @@ TEST_CASE("test_initial_noc_placement", "[noc_place_utils]") { place_ctx.block_locs.insert(ClusterBlockId(cluster_block_number), current_cluster_block_location); } - // similiar parameters for all traffic flows + // similar parameters for all traffic flows std::string source_traffic_flow_name = "test"; std::string sink_traffic_flow_name = "test_2"; double traffic_flow_latency = 10.0; @@ -135,7 +135,7 @@ TEST_CASE("test_initial_noc_placement", "[noc_place_utils]") { noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // now go and route all the traffic flows // - // start by creating the routing alagorithm + // start by creating the routing algorithm NocRouting* routing_algorithm_global = new XYRouting(); noc_ctx.noc_flows_router = routing_algorithm_global; @@ -155,12 +155,12 @@ TEST_CASE("test_initial_noc_placement", "[noc_place_utils]") { /* in the test function we will be routing all the traffic flows and then updating their link usages. So we will be generating a vector of all links * and updating their bandwidths based on the routed traffic flows. This will - * then be compared to the link badnwdith usages updated by the test function + * then be compared to the link bandwidth usages updated by the test function */ vtr::vector golden_link_bandwidths; golden_link_bandwidths.resize(noc_ctx.noc_model.get_noc_links().size(), 0.0); - // now go through the routed traffic flows and update the bandwidths of the links. Once a traffic flow has been processsed, we need to clear it so that the test function can update it with the routes it finds + // now go through the routed traffic flows and update the bandwidths of the links. Once a traffic flow has been processed, we need to clear it so that the test function can update it with the routes it finds for (int traffic_flow_number = 0; traffic_flow_number < NUM_OF_TRAFFIC_FLOWS_NOC_PLACE_UTILS_TEST; traffic_flow_number++) { const t_noc_traffic_flow& curr_traffic_flow = noc_ctx.noc_traffic_flows_storage.get_single_noc_traffic_flow((NocTrafficFlowId)traffic_flow_number); std::vector& traffic_flow_route = noc_ctx.noc_traffic_flows_storage.get_mutable_traffic_flow_route((NocTrafficFlowId)traffic_flow_number); @@ -204,7 +204,7 @@ TEST_CASE("test_initial_comp_cost_functions", "[noc_place_utils]") { auto& noc_ctx = g_vpr_ctx.mutable_noc(); auto& place_ctx = g_vpr_ctx.mutable_placement(); - // start by deleting any global datastructures (this is so that we dont have corruption from previous tests) + // start by deleting any global datastructures (this is so that we don't have corruption from previous tests) noc_ctx.noc_model.clear_noc(); noc_ctx.noc_traffic_flows_storage.clear_traffic_flows(); delete noc_ctx.noc_flows_router; @@ -259,7 +259,7 @@ TEST_CASE("test_initial_comp_cost_functions", "[noc_place_utils]") { } } - // now we need to create router cluster blocks and assing them to placed at a router hard block + // now we need to create router cluster blocks and passing them to placed at a router hard block for (int cluster_block_number = 0; cluster_block_number < NUM_OF_LOGICAL_ROUTER_BLOCKS_NOC_PLACE_UTILS_TEST; cluster_block_number++) { // since the indexes for the hard router blocks start from 0, we will just place the router clusters on hard router blocks with the same id // @@ -276,7 +276,7 @@ TEST_CASE("test_initial_comp_cost_functions", "[noc_place_utils]") { noc_ctx.noc_model.set_noc_link_latency(1); noc_ctx.noc_model.set_noc_router_latency(1); - // similiar parameters for all traffic flows + // similar parameters for all traffic flows std::string source_traffic_flow_name = "test"; std::string sink_traffic_flow_name = "test_2"; int number_of_created_traffic_flows = 0; @@ -312,12 +312,12 @@ TEST_CASE("test_initial_comp_cost_functions", "[noc_place_utils]") { noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); - // need to route all the traffic flows so create a datstructure to store them here + // need to route all the traffic flows so create a datastructure to store them here std::vector golden_traffic_flow_route_sizes; golden_traffic_flow_route_sizes.resize(number_of_created_traffic_flows); // now go and route all the traffic flows // - // start by creating the routing alagorithm + // start by creating the routing algorithm NocRouting* routing_algorithm_global = new XYRouting(); noc_ctx.noc_flows_router = routing_algorithm_global; @@ -344,7 +344,7 @@ TEST_CASE("test_initial_comp_cost_functions", "[noc_place_utils]") { } // assume this works - // this is needed to setup the global noc packet router and also global datastructures + // this is needed to set up the global noc packet router and also global datastructures initial_noc_placement(); SECTION("test_comp_noc_aggregate_bandwidth_cost") { @@ -440,7 +440,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ auto& noc_ctx = g_vpr_ctx.mutable_noc(); auto& place_ctx = g_vpr_ctx.mutable_placement(); - // start by deleting any global datastructures (this is so that we dont have corruption from previous tests) + // start by deleting any global datastructures (this is so that we don't have corruption from previous tests) noc_ctx.noc_model.clear_noc(); noc_ctx.noc_traffic_flows_storage.clear_traffic_flows(); delete noc_ctx.noc_flows_router; @@ -456,7 +456,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ int router_grid_position_y; // noc options used in this test - // we creae these randomly + // we create these randomly t_noc_opts noc_opts; noc_opts.noc_latency_constraints_weighting = dist_3(double_engine); noc_opts.noc_latency_weighting = dist_3(double_engine); @@ -507,7 +507,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ } } - // now we need to create router cluster blocks and assing them to placed at a router hard block as an initial position + // now we need to create router cluster blocks and passing them to placed at a router hard block as an initial position for (int cluster_block_number = 0; cluster_block_number < NUM_OF_LOGICAL_ROUTER_BLOCKS_NOC_PLACE_UTILS_TEST; cluster_block_number++) { // since the indexes for the hard router blocks start from 0, we will just place the router clusters on hard router blocks with the same id // @@ -523,7 +523,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ place_ctx.block_locs.insert(ClusterBlockId(cluster_block_number), current_cluster_block_location); } - // similiar parameters for all traffic flows + // similar parameters for all traffic flows std::string source_traffic_flow_name = "test"; std::string sink_traffic_flow_name = "test_2"; int number_of_created_traffic_flows = 0; @@ -538,7 +538,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ ClusterBlockId sink_router_for_traffic_flow; // randomly choose sink router - // make sure the traffic flow does not start and end at the same router and also make sure that the sink router is never that last 2 router cluster blocks (we dont want them associated to any traffic flows) + // make sure the traffic flow does not start and end at the same router and also make sure that the sink router is never that last 2 router cluster blocks (we don't want them associated to any traffic flows) do { sink_router_for_traffic_flow = (ClusterBlockId)dist(rand_num_gen); } while (sink_router_for_traffic_flow == source_router_for_traffic_flow); @@ -562,7 +562,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // now go and route all the traffic flows // - // start by creating the routing alagorithm + // start by creating the routing algorithm NocRouting* routing_algorithm_global = new XYRouting(); noc_ctx.noc_flows_router = routing_algorithm_global; @@ -594,7 +594,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ } // assume this works - // this is needed to setup the global noc packet router and also global datastructures + // this is needed to set up the global noc packet router and also global datastructures initial_noc_placement(); // datastructure below will store the bandwidth usages of all the links @@ -640,7 +640,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ std::unordered_set routed_traffic_flows; /* Now we imitate placement here by swapping two clusters block - * posistions. In each iteration, we first update the positions + * positions. In each iteration, we first update the positions * of the two router cluster blocks, then we update the traffic * flows and then the bandwidth usages of the links. Then we call * the test function and then move onto the next iteration. @@ -654,7 +654,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ swap_router_block_two = (ClusterBlockId)dist(rand_num_gen); } while (swap_router_block_one == swap_router_block_two); - //setup the moved blocks datastructure for the test function + //set up the moved blocks datastructure for the test function blocks_affected.num_moved_blocks = 2; blocks_affected.moved_blocks[0].block_num = swap_router_block_one; @@ -766,7 +766,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ * Now we will run a test where the two routers that are moved share a * traffic flow with each other. This is used to verify whether the * function under test correctly gets the positions of the moved blocks. - * It also checks that the test function correctly handles the situatio + * It also checks that the test function correctly handles the situation * where the moved router cluster block is a sink router in its associated traffic flows. */ // start by choosing a random traffic flow @@ -780,7 +780,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ ClusterBlockId swap_router_block_two = chosen_traffic_flow.source_router_cluster_id; // now perform the swap - //setup the moved blocks datastructure for the test function + //set up the moved blocks datastructure for the test function blocks_affected.num_moved_blocks = 2; blocks_affected.moved_blocks[0].block_num = swap_router_block_one; @@ -881,13 +881,13 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ * Now we will run a test where one of the router clusters we will swap has no traffic flows associated with it. This will make sure whether the test * function currently determines that a router cluster block has no traffic flows and also calculates that cost accordingly (cost of 0) */ - // start by picking one of the router cluster blocks that dont have any traffic flows as one of our cluster blocks to swap + // start by picking one of the router cluster blocks that don't have any traffic flows as one of our cluster blocks to swap swap_router_block_one = (ClusterBlockId)(NUM_OF_LOGICAL_ROUTER_BLOCKS_NOC_PLACE_UTILS_TEST - 1); // the second router block to swap will be one with a traffic flow associated to it swap_router_block_two = (ClusterBlockId)(NUM_OF_TRAFFIC_FLOWS_NOC_PLACE_UTILS_TEST - 4); // now perform the swap - //setup the moved blocks datastructure for the test function + //set up the moved blocks datastructure for the test function blocks_affected.num_moved_blocks = 2; blocks_affected.moved_blocks[0].block_num = swap_router_block_one; @@ -909,7 +909,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ place_ctx.block_locs[swap_router_block_two].loc = blocks_affected.moved_blocks[1].new_loc; // get all the associated traffic flows of the moved cluster blocks - // remember that the first cluster block doesnt have any traffic flows associated to it + // remember that the first cluster block doesn't have any traffic flows associated to it assoc_traffic_flows_block_two = noc_ctx.noc_traffic_flows_storage.get_traffic_flows_associated_to_router_block(swap_router_block_two); // this is for the second swapped block @@ -963,13 +963,13 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ * the test function currently determines that both router blocks have no * traffic flows associated with them and calculates the cost change accordingly (total cost of 0) */ - // start by picking one of the router cluster blocks that dont have any traffic flows as one of our cluster blocks to swap + // start by picking one of the router cluster blocks that don't have any traffic flows as one of our cluster blocks to swap swap_router_block_one = (ClusterBlockId)(NUM_OF_LOGICAL_ROUTER_BLOCKS_NOC_PLACE_UTILS_TEST - 1); // the second router block to swap will be one with a traffic flow associated to it swap_router_block_two = (ClusterBlockId)(NUM_OF_TRAFFIC_FLOWS_NOC_PLACE_UTILS_TEST - 2); // now perform the swap - //setup the moved blocks datastructure for the test function + //set up the moved blocks datastructure for the test function blocks_affected.num_moved_blocks = 2; blocks_affected.moved_blocks[0].block_num = swap_router_block_one; @@ -990,7 +990,7 @@ TEST_CASE("test_find_affected_noc_routers_and_update_noc_costs, test_commit_noc_ place_ctx.block_locs[swap_router_block_one].loc = blocks_affected.moved_blocks[0].new_loc; place_ctx.block_locs[swap_router_block_two].loc = blocks_affected.moved_blocks[1].new_loc; - // we dont have to calculate the costs or update bandwidths because the swapped router blocks do not have any associated traffic flows // + // we don't have to calculate the costs or update bandwidths because the swapped router blocks do not have any associated traffic flows // // reset the delta costs delta_aggr_band_cost = 0.; @@ -1129,7 +1129,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { auto& noc_ctx = g_vpr_ctx.mutable_noc(); auto& place_ctx = g_vpr_ctx.mutable_placement(); - // start by deleting any global datastructures (this is so that we dont have corruption from previous tests) + // start by deleting any global datastructures (this is so that we don't have corruption from previous tests) noc_ctx.noc_model.clear_noc(); noc_ctx.noc_traffic_flows_storage.clear_traffic_flows(); delete noc_ctx.noc_flows_router; @@ -1145,7 +1145,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { int router_grid_position_y; // noc options used in this test - // we creae these randomly + // we create these randomly t_noc_opts noc_opts; noc_opts.noc_latency_constraints_weighting = dist_3(double_engine); noc_opts.noc_latency_weighting = dist_3(double_engine); @@ -1193,7 +1193,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { } } - // now we need to create router cluster blocks and assing them to placed at a router hard block as an initial position + // now we need to create router cluster blocks and passing them to placed at a router hard block as an initial position for (int cluster_block_number = 0; cluster_block_number < NUM_OF_LOGICAL_ROUTER_BLOCKS_NOC_PLACE_UTILS_TEST; cluster_block_number++) { // since the indexes for the hard router blocks start from 0, we will just place the router clusters on hard router blocks with the same id // @@ -1209,7 +1209,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { place_ctx.block_locs.insert(ClusterBlockId(cluster_block_number), current_cluster_block_location); } - // similiar parameters for all traffic flows + // similar parameters for all traffic flows std::string source_traffic_flow_name = "test"; std::string sink_traffic_flow_name = "test_2"; int number_of_created_traffic_flows = 0; @@ -1247,7 +1247,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // now go and route all the traffic flows // - // start by creating the routing alagorithm + // start by creating the routing algorithm NocRouting* routing_algorithm_global = new XYRouting(); noc_ctx.noc_flows_router = routing_algorithm_global; @@ -1271,7 +1271,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { } // assume this works - // this is needed to setup the global noc packet router and also global datastructures + // this is needed to set up the global noc packet router and also global datastructures initial_noc_placement(); // datastructure below will store the bandwidth usages of all the links @@ -1314,8 +1314,8 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { swap_router_block_two = (ClusterBlockId)dist(rand_num_gen); } while (swap_router_block_one == swap_router_block_two); - //setup the moved blocks datastructure for the test function - // this is needed for the test function (it needs to know what blocks were swapped so it can undo it) + //set up the moved blocks datastructure for the test function + // this is needed for the test function (it needs to know what blocks were swapped, so it can undo it) blocks_affected.num_moved_blocks = 2; blocks_affected.moved_blocks[0].block_num = swap_router_block_one; @@ -1346,7 +1346,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { traffic_flow_route.clear(); // go through the current traffic flow and reduce the bandwidths of the links (we only update this in the NoC, since these changes should be rectified by the test function) - // This shouldnt be updated in the golden bandwidths since we are imitating a swap of blocks and not having a real swap of blocks + // This shouldn't be updated in the golden bandwidths since we are imitating a swap of blocks and not having a real swap of blocks for (auto& link : golden_traffic_flow_routes[traffic_flow]) { // update the link bandwidth in the NoC datastructure double current_link_bandwidth = noc_ctx.noc_model.get_single_noc_link(link).get_bandwidth_usage(); @@ -1357,7 +1357,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { noc_ctx.noc_flows_router->route_flow(router_where_cluster_is_placed[curr_traffic_flow.source_router_cluster_id], router_where_cluster_is_placed[curr_traffic_flow.sink_router_cluster_id], golden_traffic_flow_routes[traffic_flow], noc_ctx.noc_model); // go through the current traffic flow and reduce the bandwidths of the links (we only update this in the NoC, since these changes should be rectified by the test function) - // This shouldnt be updated in the golden bandwidths since we are imitating a swap of blocks and not having a real swap of blocks + // This shouldn't be updated in the golden bandwidths since we are imitating a swap of blocks and not having a real swap of blocks for (auto& link : golden_traffic_flow_routes[traffic_flow]) { // update the link bandwidth in the NoC datastructure double current_link_bandwidth = noc_ctx.noc_model.get_single_noc_link(link).get_bandwidth_usage(); @@ -1379,7 +1379,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { traffic_flow_route.clear(); // go through the current traffic flow and reduce the bandwidths of the links (we only update this in the NoC, since these changes should be rectified by the test function) - // This shouldnt be updated in the golden bandwidths since we are imitating a swap of blocks and not having a real swap of blocks + // This shouldn't be updated in the golden bandwidths since we are imitating a swap of blocks and not having a real swap of blocks for (auto& link : golden_traffic_flow_routes[traffic_flow]) { // update the link bandwidth in the NoC datastructure double current_link_bandwidth = noc_ctx.noc_model.get_single_noc_link(link).get_bandwidth_usage(); @@ -1390,7 +1390,7 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { noc_ctx.noc_flows_router->route_flow(router_where_cluster_is_placed[curr_traffic_flow.source_router_cluster_id], router_where_cluster_is_placed[curr_traffic_flow.sink_router_cluster_id], golden_traffic_flow_routes[traffic_flow], noc_ctx.noc_model); // go through the current traffic flow and reduce the bandwidths of the links (we only update this in the NoC, since these changes should be rectified by the test function) - // This shouldnt be updated in the golden bandwidths since we are imitating a swap of blocks and not having a real swap of blocks + // This shouldn't be updated in the golden bandwidths since we are imitating a swap of blocks and not having a real swap of blocks for (auto& link : golden_traffic_flow_routes[traffic_flow]) { // update the link bandwidth in the NoC datastructure double current_link_bandwidth = noc_ctx.noc_model.get_single_noc_link(link).get_bandwidth_usage(); @@ -1402,8 +1402,8 @@ TEST_CASE("test_revert_noc_traffic_flow_routes", "[noc_place_utils]") { } } - // okay so now we undo the swapped blocks. We dont need to update the block locations in the placement datastructure since we initially never moved the blocks that were mswapped at the start. - // To undoe this we just need to update the noc link bandwidths as if there was no swap (we do this by calling the test function) + // okay so now we undo the swapped blocks. We don't need to update the block locations in the placement datastructure since we initially never moved the blocks that were swapped at the start. + // To undo this we just need to update the noc link bandwidths as if there was no swap (we do this by calling the test function) // This should then re-update the noc link bandwidths to their values before we imitated the swap above // THe result is that the link bandwidths should match the golden link bandwidths that never changed after the initial router block placement (at a point before block swapping) revert_noc_traffic_flow_routes(blocks_affected); @@ -1436,7 +1436,7 @@ TEST_CASE("test_check_noc_placement_costs", "[noc_place_utils]") { auto& noc_ctx = g_vpr_ctx.mutable_noc(); auto& place_ctx = g_vpr_ctx.mutable_placement(); - // start by deleting any global datastructures (this is so that we dont have corruption from previous tests) + // start by deleting any global datastructures (this is so that we don't have corruption from previous tests) noc_ctx.noc_model.clear_noc(); noc_ctx.noc_traffic_flows_storage.clear_traffic_flows(); delete noc_ctx.noc_flows_router; @@ -1459,7 +1459,7 @@ TEST_CASE("test_check_noc_placement_costs", "[noc_place_utils]") { double router_latency = 1; // noc options used in this test - // we creae these randomly + // we create these randomly t_noc_opts noc_opts; noc_opts.noc_latency_constraints_weighting = dist_3(double_engine); noc_opts.noc_latency_weighting = dist_3(double_engine); @@ -1508,7 +1508,7 @@ TEST_CASE("test_check_noc_placement_costs", "[noc_place_utils]") { } } - // now we need to create router cluster blocks and assing them to placed at a router hard block as an initial position + // now we need to create router cluster blocks and passing them to placed at a router hard block as an initial position for (int cluster_block_number = 0; cluster_block_number < NUM_OF_LOGICAL_ROUTER_BLOCKS_NOC_PLACE_UTILS_TEST; cluster_block_number++) { // since the indexes for the hard router blocks start from 0, we will just place the router clusters on hard router blocks with the same id // @@ -1524,7 +1524,7 @@ TEST_CASE("test_check_noc_placement_costs", "[noc_place_utils]") { place_ctx.block_locs.insert(ClusterBlockId(cluster_block_number), current_cluster_block_location); } - // similiar parameters for all traffic flows + // similar parameters for all traffic flows std::string source_traffic_flow_name = "test"; std::string sink_traffic_flow_name = "test_2"; int number_of_created_traffic_flows = 0; @@ -1562,7 +1562,7 @@ TEST_CASE("test_check_noc_placement_costs", "[noc_place_utils]") { noc_ctx.noc_traffic_flows_storage.finished_noc_traffic_flows_setup(); // now go and route all the traffic flows // - // start by creating the routing alagorithm + // start by creating the routing algorithm NocRouting* routing_algorithm_global = new XYRouting(); noc_ctx.noc_flows_router = routing_algorithm_global; @@ -1609,7 +1609,7 @@ TEST_CASE("test_check_noc_placement_costs", "[noc_place_utils]") { costs.noc_latency_cost += curr_latency_cost; } - // this defines the error tolerance that is allowed between the golden noc costs and the costs found by the test funcion: check_noc_placement_costs + // this defines the error tolerance that is allowed between the golden noc costs and the costs found by the test function: check_noc_placement_costs // we will set it to what the VTR placer uses double error_tolerance = .01; @@ -1621,7 +1621,7 @@ TEST_CASE("test_check_noc_placement_costs", "[noc_place_utils]") { REQUIRE(error == 0); } SECTION("Case where the check place fails for both NoC costs") { - // we need to make the aggregate badnwdith cost and latency cost be a value that is larger or smaller than the tolerance value + // we need to make the aggregate bandwidth cost and latency cost be a value that is larger or smaller than the tolerance value costs.noc_aggregate_bandwidth_cost += (costs.noc_aggregate_bandwidth_cost * error_tolerance * 2); costs.noc_latency_cost -= (costs.noc_latency_cost * error_tolerance * 2); From 821544f6f6c82b02c8a220a2665500eeedd85979 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 17:31:45 -0400 Subject: [PATCH 24/29] fixed comment typos in test_noc_storage --- vpr/test/test_noc_storage.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/vpr/test/test_noc_storage.cpp b/vpr/test/test_noc_storage.cpp index b676cd4ed1f..8ceec3f36e1 100644 --- a/vpr/test/test_noc_storage.cpp +++ b/vpr/test/test_noc_storage.cpp @@ -52,7 +52,7 @@ TEST_CASE("test_adding_routers_to_noc_storage", "[vpr_noc]") { // now verify that the routers were added properly by reading the routers back from the noc and comparing them to the golden set for (int router_number = 0; router_number < NUM_OF_ROUTERS; router_number++) { - // get the converted router id and the corresponsing router to verify + // get the converted router id and the corresponding router to verify // no need to run a conversion, the router id mapping is 1 to 1 converted_id = (NocRouterId)router_number; const NocRouter& router_to_verify = test_noc.get_single_noc_router(converted_id); @@ -104,7 +104,7 @@ TEST_CASE("test_router_id_conversion", "[vpr_noc]") { // now verify that the routers were added properly by reading the routers back from the noc and comparing them to the golden set for (int router_number = 0; router_number < NUM_OF_ROUTERS; router_number++) { - // get the converted router id and the corresponsing router to verify + // get the converted router id and the corresponding router to verify converted_id = test_noc.convert_router_id(router_number); const NocRouter& router_to_verify = test_noc.get_single_noc_router(converted_id); @@ -183,7 +183,7 @@ TEST_CASE("test_add_link", "[vpr_noc]") { const NocRouter& test_link_source_router = test_noc.get_single_noc_router(current_link_to_test.get_source_router()); const NocRouter& test_link_sink_router = test_noc.get_single_noc_router(current_link_to_test.get_sink_router()); - // now get the source and sink routers of the golde link + // now get the source and sink routers of the golden link const NocRouter& golden_link_source_router = test_noc.get_single_noc_router(golden_set[link_number].get_source_router()); const NocRouter& golden_link_sink_router = test_noc.get_single_noc_router(golden_set[link_number].get_sink_router()); @@ -290,7 +290,7 @@ TEST_CASE("test_remove_link", "[vpr_noc]") { NocRouterId source; NocRouterId sink; - // create routers and add it to the NoC + // create routers and add it to the // noc router stuff (we need routers before being able to add links) int router_id = 0; int curr_router_x_pos = 0; @@ -342,7 +342,7 @@ TEST_CASE("test_remove_link", "[vpr_noc]") { bool link_removed_from_outgoing_vector = true; auto& outgoing_links = test_noc.get_noc_router_connections(link_to_remove_src_router); - // go through all the outgoing links of the source router in the link we removed and check that the link does not exis there as well. + // go through all the outgoing links of the source router in the link we removed and check that the link does not exist there as well. for (auto outgoing_link_id = outgoing_links.begin(); outgoing_link_id != outgoing_links.end(); outgoing_link_id++) { // get the current outgoing link const NocLink& curr_outgoing_link = test_noc.get_single_noc_link(*outgoing_link_id); @@ -415,7 +415,7 @@ TEST_CASE("test_generate_router_key_from_grid_location", "[vpr_noc]") { router_grid_position_x = router_number; router_grid_position_y = router_number; - // add the current router_id to the golden vector (the id is determined similiar to how it is done in add_router()) + // add the current router_id to the golden vector (the id is determined similar to how it is done in add_router()) // this vector is built so that the index represents the grid location of the current router golden_set.emplace_back((NocRouterId)router_number); @@ -427,7 +427,7 @@ TEST_CASE("test_generate_router_key_from_grid_location", "[vpr_noc]") { // the grid locations go from 0 to the total number of routers in the NoC for (int grid_location = 0; grid_location < NUM_OF_ROUTERS; grid_location++) { // contains the grid location of a router block seen during placement - // we dont care about the subtile so give it a arbritary value + // we don't care about the subtile so give it an arbitrary value t_pl_loc placement_router_grid_location = t_pl_loc(grid_location, grid_location, -1); NocRouterId found_router_at_grid_location = test_noc.get_router_at_grid_location(placement_router_grid_location); From 5ad822fec95c77d40411791fc01a1cf87bd9b19a Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Mon, 15 May 2023 17:33:21 -0400 Subject: [PATCH 25/29] fixed typos in test_noc_traffic_flows --- vpr/test/test_noc_traffic_flows.cpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/vpr/test/test_noc_traffic_flows.cpp b/vpr/test/test_noc_traffic_flows.cpp index b258af39957..30791599805 100644 --- a/vpr/test/test_noc_traffic_flows.cpp +++ b/vpr/test/test_noc_traffic_flows.cpp @@ -15,14 +15,14 @@ TEST_CASE("test_adding_traffic_flows", "[vpr_noc_traffic_flows]") { // parameters for each traffic flow std::string source_router_name = "test_1"; - std::string sink_router_nanme = "test_2"; + std::string sink_router_name = "test_2"; double traffic_flow_bandwidth = 200; double traffic_flow_latency = 10; int traffic_flow_priority = 1; ClusterBlockId source_router_id; ClusterBlockId sink_router_id; NocTrafficFlowId curr_flow_id; - // setup the test data + // set up the test data // create all the routers std::vector golden_router_blocks_list; @@ -40,7 +40,7 @@ TEST_CASE("test_adding_traffic_flows", "[vpr_noc_traffic_flows]") { for (int router = 0; router < NUM_OF_ROUTERS; router++) { for (int second_router = 0; second_router < NUM_OF_ROUTERS; second_router++) { - // dont want the case where the source and destination routers are the same + // don't want the case where the source and destination routers are the same if (router == second_router) { continue; } @@ -49,7 +49,7 @@ TEST_CASE("test_adding_traffic_flows", "[vpr_noc_traffic_flows]") { sink_router_id = (ClusterBlockId)second_router; // need to match how the test function does it - golden_traffic_flow_list.emplace_back(source_router_name, sink_router_nanme, source_router_id, sink_router_id, traffic_flow_bandwidth, traffic_flow_latency, traffic_flow_priority); + golden_traffic_flow_list.emplace_back(source_router_name, sink_router_name, source_router_id, sink_router_id, traffic_flow_bandwidth, traffic_flow_latency, traffic_flow_priority); curr_flow_id = (NocTrafficFlowId)(golden_traffic_flow_list.size() - 1); @@ -59,12 +59,12 @@ TEST_CASE("test_adding_traffic_flows", "[vpr_noc_traffic_flows]") { } } - // finished settting up all the golden information, so now perform the tests + // finished setting up all the golden information, so now perform the tests SECTION("Verifying that all created traffic flows and their related information are stored correctly.") { // add all the traffic flows to the datastructure for (int router = 0; router < NUM_OF_ROUTERS; router++) { for (int second_router = 0; second_router < NUM_OF_ROUTERS; second_router++) { - // dont want the case where the source and destination routers are the same + // don't want the case where the source and destination routers are the same if (router == second_router) { continue; } @@ -73,7 +73,7 @@ TEST_CASE("test_adding_traffic_flows", "[vpr_noc_traffic_flows]") { sink_router_id = (ClusterBlockId)second_router; // create and add the traffic flow - traffic_flow_storage.create_noc_traffic_flow(source_router_name, sink_router_nanme, source_router_id, sink_router_id, traffic_flow_bandwidth, traffic_flow_latency, traffic_flow_priority); + traffic_flow_storage.create_noc_traffic_flow(source_router_name, sink_router_name, source_router_id, sink_router_id, traffic_flow_bandwidth, traffic_flow_latency, traffic_flow_priority); } } @@ -81,7 +81,7 @@ TEST_CASE("test_adding_traffic_flows", "[vpr_noc_traffic_flows]") { // check the set of routers first to see that they were all added properly for (int router = 0; router < size_of_router_block_list; router++) { - // every router in the golden list needs to exist in the traffic flow datastructure (this also tests cases where a router was added multiple times, this shouldnt affect it) + // every router in the golden list needs to exist in the traffic flow datastructure (this also tests cases where a router was added multiple times, this shouldn't affect it) REQUIRE(traffic_flow_storage.check_if_cluster_block_has_traffic_flows(golden_router_blocks_list[router]) == true); } @@ -106,7 +106,7 @@ TEST_CASE("test_adding_traffic_flows", "[vpr_noc_traffic_flows]") { // get the traffic flows associated to the current router from the test datastructure const std::vector* associated_traffic_flows_to_router = traffic_flow_storage.get_traffic_flows_associated_to_router_block(router_id); - // make sure that the number of traffic flows associated to each router within the Noctrafficflows datastrcuture matches the golden set + // make sure that the number of traffic flows associated to each router within the NocTrafficFlows data structure matches the golden set REQUIRE((int)associated_traffic_flows_to_router->size() == number_of_traffic_flows_associated_with_current_router); // now go through the associated traffic flows and make sure the correct ones were added to the current router @@ -119,14 +119,14 @@ TEST_CASE("test_adding_traffic_flows", "[vpr_noc_traffic_flows]") { REQUIRE(NUM_OF_ROUTERS == traffic_flow_storage.get_number_of_routers_used_in_traffic_flows()); } SECTION("Checking to see if invalid blocks that are not routers exist in NocTrafficFlows.") { - // create a invalid block id + // create an invalid block id ClusterBlockId invalid_block = (ClusterBlockId)(NUM_OF_ROUTERS + 1); - // check that this block doesnt exist in the traffic flow datastructure + // check that this block doesn't exist in the traffic flow datastructure REQUIRE(traffic_flow_storage.check_if_cluster_block_has_traffic_flows(invalid_block) == false); } SECTION("Checking that when a router has no traffic flows associated to it, then the associated traffic flows vector retrieved from the NocTrafficFlows class for this router should be null.") { - // create a invalid block id (mimics the effect where a router has no traffic flows associated with it) + // create an invalid block id (mimics the effect where a router has no traffic flows associated with it) ClusterBlockId invalid_block = (ClusterBlockId)(NUM_OF_ROUTERS + 1); // check that this router has no traffic flows associated with it From ddcb510adaddbc3add6bb10afabbd910f712cfce Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 18 May 2023 13:39:48 -0400 Subject: [PATCH 26/29] noc functions comments updated based on pr review request --- vpr/src/noc/noc_traffic_flows.cpp | 3 ++- vpr/src/noc/noc_traffic_flows.h | 17 ++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index b0f132b26d6..33deea75bdb 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -71,7 +71,8 @@ void NocTrafficFlows::create_noc_traffic_flow(std::string source_router_module_n return; } -void NocTrafficFlows::set_router_cluster_in_netlist(std::vector routers_cluster_id_in_netlist) { +void NocTrafficFlows::set_router_cluster_in_netlist(std::vector& routers_cluster_id_in_netlist) { + router_cluster_in_netlist.clear(); //copy the input vector to the internal vector for (auto router_id : routers_cluster_id_in_netlist) { router_cluster_in_netlist.emplace_back(router_id); diff --git a/vpr/src/noc/noc_traffic_flows.h b/vpr/src/noc/noc_traffic_flows.h index 2ec4edc73f0..befd6c0554b 100644 --- a/vpr/src/noc/noc_traffic_flows.h +++ b/vpr/src/noc/noc_traffic_flows.h @@ -213,8 +213,8 @@ class NocTrafficFlows { std::vector& get_mutable_traffic_flow_route(NocTrafficFlowId traffic_flow_id); /** - * @return provides access to a vector containing all logical router ClusterBlockId to - * help "propose_router_swap" function to choose a random logical router to move. + * @return a vector ([0..num_logical_router-1]) where each entry gives the clusterBlockId + * of a logical NoC router. Used for fast lookups in the placer. */ const std::vector& get_router_clusters_in_netlist(void) const; @@ -255,19 +255,14 @@ class NocTrafficFlows { void create_noc_traffic_flow(std::string source_router_module_name, std::string sink_router_module_name, ClusterBlockId source_router_cluster_id, ClusterBlockId sink_router_cluster_id, double traffic_flow_bandwidth, double traffic_flow_latency, int traffic_flow_priority); /** - * @brief fill "router_cluster_in_netlist" vector which is an internal - * data structure used in "propose_router_swap" function. This vector - * will contain NoC routers' ClusterBlockId specified in the netlist to - * simplify their quick access during choosing a random router from netlist. - * This routine is executed only once during traffic flow extraction - * after "get_cluster_blocks_compatible_with_noc_router_tiles" - * function and the vector will be constant afterward. + * @brief Copies the passed in router_cluster_id_in_netlist vector to the + * private internal vector. * - * @param routers_cluster_id_in_netlist A vector containing all routers' + * @param routers_cluster_id_in_netlist A vector ([0..num_logical_routers-1]) containing all routers' * ClusterBlockId extracted from netlist. * */ - void set_router_cluster_in_netlist(std::vector routers_cluster_id_in_netlist); + void set_router_cluster_in_netlist(std::vector& routers_cluster_id_in_netlist); //utility functions From dec2295aa66881f913b8a8f841dd00944155ab94 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 18 May 2023 13:46:41 -0400 Subject: [PATCH 27/29] noc_place_utils functions review applied --- vpr/src/place/noc_place_utils.cpp | 4 ++-- vpr/src/place/noc_place_utils.h | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/vpr/src/place/noc_place_utils.cpp b/vpr/src/place/noc_place_utils.cpp index 3b374ea8333..0a53e936d51 100644 --- a/vpr/src/place/noc_place_utils.cpp +++ b/vpr/src/place/noc_place_utils.cpp @@ -109,14 +109,14 @@ std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, return curr_traffic_flow_route; } -void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, int how_to_update_links, double traffic_flow_bandwidth) { +void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, int inc_or_dec, double traffic_flow_bandwidth) { // go through the links within the traffic flow route and update their bandwidth usage for (auto& link_in_route_id : traffic_flow_route) { // get the link to update and its current bandwidth NocLink& curr_link = noc_model.get_single_mutable_noc_link(link_in_route_id); double curr_link_bandwidth = curr_link.get_bandwidth_usage(); - curr_link.set_bandwidth_usage(curr_link_bandwidth + how_to_update_links * traffic_flow_bandwidth); + curr_link.set_bandwidth_usage(curr_link_bandwidth + inc_or_dec * traffic_flow_bandwidth); // check that the bandwidth never goes to negative VTR_ASSERT(curr_link.get_bandwidth_usage() >= 0.0); diff --git a/vpr/src/place/noc_place_utils.h b/vpr/src/place/noc_place_utils.h index dde087ad40c..f001dfce993 100644 --- a/vpr/src/place/noc_place_utils.h +++ b/vpr/src/place/noc_place_utils.h @@ -26,8 +26,8 @@ constexpr double MIN_EXPECTED_NOC_LATENCY_COST = 1.e-12; /** * @brief Each traffic flow cost consists of two components: - * 1) traffic flow aggregate bandwidth - * 2) traffic flow latency + * 1) traffic flow aggregate bandwidth (sum over all used links of the traffic flow bandwidth) + * 2) traffic flow latency (currently unloaded/best-case latency of the flow) * NoC placement code will keep an array-of-struct to easily access each * traffic flow cost. */ @@ -142,14 +142,14 @@ std::vector& get_traffic_flow_route(NocTrafficFlowId traffic_flow_id, * contains a collection of links in the NoC. * @param noc_model Contains all the links and routers within the NoC. Used * to update link information. - * @param how_to_update_links Determines how the bandwidths of links found + * @param inc_or_dec Determines how the bandwidths of links found * in the traffic flow route are updated. If it is -1, the route flow has * been removed and links' bandwidth must be decremented. Otherwise, the a traffic * flow has been re-routed and its links' bandwidth should be incremented. * @param traffic_flow_bandwidth The bandwidth of a traffic flow. This will * be used to update bandwidth usage of the links. */ -void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, int how_to_update_links, double traffic_flow_bandwidth); +void update_traffic_flow_link_usage(const std::vector& traffic_flow_route, NocStorage& noc_model, int inc_or_dec, double traffic_flow_bandwidth); /** * @brief Goes through all the traffic flows associated to a moved From 8641df8b6f892885da5da38aa3edbfc43263a88c Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 18 May 2023 17:26:21 -0400 Subject: [PATCH 28/29] changed the setter function argument to a const value --- vpr/src/noc/noc_traffic_flows.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vpr/src/noc/noc_traffic_flows.h b/vpr/src/noc/noc_traffic_flows.h index befd6c0554b..dae52b5184d 100644 --- a/vpr/src/noc/noc_traffic_flows.h +++ b/vpr/src/noc/noc_traffic_flows.h @@ -262,7 +262,7 @@ class NocTrafficFlows { * ClusterBlockId extracted from netlist. * */ - void set_router_cluster_in_netlist(std::vector& routers_cluster_id_in_netlist); + void set_router_cluster_in_netlist(const std::vector& routers_cluster_id_in_netlist); //utility functions From cbc5b0b3e4d9a98fafe11fc7156db62bde47afc9 Mon Sep 17 00:00:00 2001 From: saaramahmoudi Date: Thu, 18 May 2023 17:40:46 -0400 Subject: [PATCH 29/29] added the const to cpp file --- vpr/src/noc/noc_traffic_flows.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vpr/src/noc/noc_traffic_flows.cpp b/vpr/src/noc/noc_traffic_flows.cpp index 33deea75bdb..50e323cb716 100644 --- a/vpr/src/noc/noc_traffic_flows.cpp +++ b/vpr/src/noc/noc_traffic_flows.cpp @@ -71,7 +71,7 @@ void NocTrafficFlows::create_noc_traffic_flow(std::string source_router_module_n return; } -void NocTrafficFlows::set_router_cluster_in_netlist(std::vector& routers_cluster_id_in_netlist) { +void NocTrafficFlows::set_router_cluster_in_netlist(const std::vector& routers_cluster_id_in_netlist) { router_cluster_in_netlist.clear(); //copy the input vector to the internal vector for (auto router_id : routers_cluster_id_in_netlist) {