Merge pull request #2315 from verilog-to-routing/noc_refactor

NoC Placement Code Improvement

Author: vaughnbetz

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.

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);

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

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

vpr/src/noc/noc_storage.cpp

@@ -70,23 +70,23 @@ 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));
     router_id_conversion_table.emplace(id, converted_id);
 
     /* 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<int, NocRouterId>(router_key, converted_id));
 
     return;
@@ -127,20 +127,20 @@ 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<NocLinkId>* 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.
         // Save this link index and remove it
         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());

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<int, NocRouterId> 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,15 +336,15 @@ 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
      * links as this will require a re-indexing of all link ids. Instead,
      * 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,17 +431,17 @@ 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.
      */
     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.
      * 

vpr/src/noc/noc_traffic_flows.cpp

@@ -44,10 +44,14 @@ std::vector<NocLinkId>& NocTrafficFlows::get_mutable_traffic_flow_route(NocTraff
     return traffic_flow_routes[traffic_flow_id];
 }
 
-const std::unordered_set<ClusterBlockId>& NocTrafficFlows::get_router_clusters_in_netlist(void) const {
+const std::vector<ClusterBlockId>& NocTrafficFlows::get_router_clusters_in_netlist(void) const {
     return router_cluster_in_netlist;
 }
 
+const std::vector<NocTrafficFlowId>& 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,22 +62,26 @@ 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);
     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;
 }
 
+void NocTrafficFlows::set_router_cluster_in_netlist(const std::vector<ClusterBlockId>& 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);
+    }
+}
+
 // 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;
 
@@ -87,6 +95,7 @@ void NocTrafficFlows::finshed_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();
@@ -100,7 +109,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);
 }
 
@@ -110,7 +119,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<ClusterBlockId, std::vector<NocTrafficFlowId>>(associated_router_id, {traffic_flow_id}));
@@ -150,7 +159,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
@@ -177,7 +186,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");
     }