return by value in parse_direct_pin_name()

soheilshahrouz · soheilshahrouz · commit 6d722f3c34ae · 2024-10-22T16:30:39.000-04:00
diff --git a/libs/libarchfpga/src/physical_types_util.h b/libs/libarchfpga/src/physical_types_util.h
@@ -284,7 +284,7 @@ int get_sub_tile_physical_pin(int sub_tile_index,
  * Given that each sub_tile's port that has exactly the same name has to be equivalent
  * one to the other, it is indifferent which port is returned.
  */
-t_physical_tile_port find_tile_port_by_name(t_physical_tile_type_ptr type, const char* port_name);
+t_physical_tile_port find_tile_port_by_name(t_physical_tile_type_ptr type, std::string_view port_name);
 
 /**
  * @brief Returns the physical tile port given the port name and the corresponding sub tile
diff --git a/vpr/src/place/place_macro.cpp b/vpr/src/place/place_macro.cpp
@@ -5,6 +5,7 @@
 #include <cmath>
 #include <sstream>
 #include <map>
+#include <string_view>
 
 #include "vtr_assert.h"
 #include "vtr_util.h"
@@ -28,11 +29,11 @@ static bool try_combine_macros(std::vector<std::vector<ClusterBlockId>>& pl_macr
  * Otherwise, mark down all the pins in that port.                                 */
 static void mark_direct_of_ports(int idirect,
                                  int direct_type,
-                                 char* pb_type_name,
-                                 char* port_name,
+                                 std::string_view pb_type_name,
+                                 std::string_view port_name,
                                  int end_pin_index,
                                  int start_pin_index,
-                                 const char* src_string,
+                                 std::string_view src_string,
                                  int line,
                                  std::vector<std::vector<int>>& idirect_from_blk_pin,
                                  std::vector<std::vector<int>>& direct_type_from_blk_pin);
@@ -50,7 +51,7 @@ static void mark_direct_of_pins(int start_pin_index,
                                 std::vector<std::vector<int>>& direct_type_from_blk_pin,
                                 int direct_type,
                                 int line,
-                                const char* src_string);
+                                std::string_view src_string);
 
 const std::vector<t_pl_macro>& PlaceMacros::macros() const {
     return pl_macros_;
@@ -393,18 +394,6 @@ int PlaceMacros::get_imacro_from_iblk(ClusterBlockId iblk) const {
 void PlaceMacros::alloc_and_load_idirect_from_blk_pin_(const std::vector<t_direct_inf>& directs) {
     const auto& device_ctx = g_vpr_ctx.device();
 
-    constexpr size_t MAX_STRING_LEN = 512;
-
-    char to_pb_type_name[MAX_STRING_LEN + 1];
-    char to_port_name[MAX_STRING_LEN + 1];
-    char from_pb_type_name[MAX_STRING_LEN + 1];
-    char from_port_name[MAX_STRING_LEN + 1];
-
-    int to_start_pin_index = -1;
-    int to_end_pin_index = -1;
-    int from_start_pin_index = -1;
-    int from_end_pin_index = -1;
-
     // Allocate and initialize the values to OPEN (-1).
     idirect_from_blk_pin_.resize(device_ctx.physical_tile_types.size());
     direct_type_from_blk_pin_.resize(device_ctx.physical_tile_types.size());
@@ -421,27 +410,27 @@ void PlaceMacros::alloc_and_load_idirect_from_blk_pin_(const std::vector<t_direc
     // Go through directs and find pins with possible direct connections
     for (size_t idirect = 0; idirect < directs.size(); idirect++) {
         // Parse out the pb_type and port name, possibly pin_indices from from_pin
-        parse_direct_pin_name(directs[idirect].from_pin.c_str(), directs[idirect].line,
-                              &from_end_pin_index, &from_start_pin_index, from_pb_type_name, from_port_name);
+        auto [from_end_pin_index, from_start_pin_index, from_pb_type_name, from_port_name] = parse_direct_pin_name(directs[idirect].from_pin,
+                                                                                                                   directs[idirect].line);
 
         // Parse out the pb_type and port name, possibly pin_indices from to_pin
-        parse_direct_pin_name(directs[idirect].to_pin.c_str(), directs[idirect].line,
-                              &to_end_pin_index, &to_start_pin_index, to_pb_type_name, to_port_name);
+        auto [to_end_pin_index, to_start_pin_index, to_pb_type_name, to_port_name] = parse_direct_pin_name(directs[idirect].to_pin,
+                                                                                                           directs[idirect].line);
 
-        /* Now I have all the data that I need, I could go through all the block pins   *
-         * in all the blocks to find all the pins that could have possible direct       *
-         * connections. Mark all down all those pins with the idirect the pins belong   *
-         * to and whether it is a source or a sink of the direct connection.            */
+        /* Now I have all the data that I need, I could go through all the block pins
+         * in all the blocks to find all the pins that could have possible direct
+         * connections. Mark all down all those pins with the idirect the pins belong
+         * to and whether it is a source or a sink of the direct connection. */
 
         // Find blocks with the same name as from_pb_type_name and from_port_name
         mark_direct_of_ports(idirect, SOURCE, from_pb_type_name, from_port_name,
-                             from_end_pin_index, from_start_pin_index, directs[idirect].from_pin.c_str(),
+                             from_end_pin_index, from_start_pin_index, directs[idirect].from_pin,
                              directs[idirect].line,
                              idirect_from_blk_pin_, direct_type_from_blk_pin_);
 
         // Then, find blocks with the same name as to_pb_type_name and from_port_name
         mark_direct_of_ports(idirect, SINK, to_pb_type_name, to_port_name,
-                             to_end_pin_index, to_start_pin_index, directs[idirect].to_pin.c_str(),
+                             to_end_pin_index, to_start_pin_index, directs[idirect].to_pin,
                              directs[idirect].line,
                              idirect_from_blk_pin_, direct_type_from_blk_pin_);
 
@@ -450,11 +439,11 @@ void PlaceMacros::alloc_and_load_idirect_from_blk_pin_(const std::vector<t_direc
 
 static void mark_direct_of_ports(int idirect,
                                  int direct_type,
-                                 char* pb_type_name,
-                                 char* port_name,
+                                 std::string_view pb_type_name,
+                                 std::string_view port_name,
                                  int end_pin_index,
                                  int start_pin_index,
-                                 const char* src_string,
+                                 std::string_view src_string,
                                  int line,
                                  std::vector<std::vector<int>>& idirect_from_blk_pin,
                                  std::vector<std::vector<int>>& direct_type_from_blk_pin) {
@@ -470,14 +459,14 @@ static void mark_direct_of_ports(int idirect,
     for (int itype = 1; itype < (int)device_ctx.physical_tile_types.size(); itype++) {
         auto& physical_tile = device_ctx.physical_tile_types[itype];
         // Find blocks with the same pb_type_name
-        if (strcmp(physical_tile.name, pb_type_name) == 0) {
+        if (pb_type_name == physical_tile.name ) {
             int num_sub_tiles = physical_tile.sub_tiles.size();
             for (int isub_tile = 0; isub_tile < num_sub_tiles; isub_tile++) {
                 auto& ports = physical_tile.sub_tiles[isub_tile].ports;
                 int num_ports = ports.size();
                 for (int iport = 0; iport < num_ports; iport++) {
                     // Find ports with the same port_name
-                    if (strcmp(ports[iport].name, port_name) == 0) {
+                    if (port_name == ports[iport].name ) {
                         int num_port_pins = ports[iport].num_pins;
 
                         // Check whether the end_pin_index is valid
@@ -517,7 +506,7 @@ static void mark_direct_of_pins(int start_pin_index,
                                 std::vector<std::vector<int>>& direct_type_from_blk_pin,
                                 int direct_type,
                                 int line,
-                                const char* src_string) {
+                                std::string_view src_string) {
     /* Mark the pin entry in idirect_from_blk_pin with idirect and the pin entry in    *
      * direct_type_from_blk_pin with direct_type from start_pin_index to               *
      * end_pin_index.                                                                  */
diff --git a/vpr/src/route/rr_graph.cpp b/vpr/src/route/rr_graph.cpp
@@ -4347,8 +4347,6 @@ static void build_unidir_rr_opins(RRGraphBuilder& rr_graph_builder,
  */
 static t_clb_to_clb_directs* alloc_and_load_clb_to_clb_directs(const std::vector<t_direct_inf>& directs, int delayless_switch) {
     t_clb_to_clb_directs* clb_to_clb_directs;
-    char *tile_name, *port_name;
-    int start_pin_index, end_pin_index;
     t_physical_tile_type_ptr physical_tile = nullptr;
     t_physical_tile_port tile_port;
 
@@ -4357,9 +4355,6 @@ static t_clb_to_clb_directs* alloc_and_load_clb_to_clb_directs(const std::vector
     const int num_directs = directs.size();
     clb_to_clb_directs = new t_clb_to_clb_directs[num_directs];
 
-    tile_name = nullptr;
-    port_name = nullptr;
-
     for (int i = 0; i < num_directs; i++) {
         //clb_to_clb_directs[i].from_clb_type;
         clb_to_clb_directs[i].from_clb_pin_start_index = 0;
@@ -4369,23 +4364,20 @@ static t_clb_to_clb_directs* alloc_and_load_clb_to_clb_directs(const std::vector
         clb_to_clb_directs[i].to_clb_pin_end_index = 0;
         clb_to_clb_directs[i].switch_index = 0;
 
-        tile_name = new char[directs[i].from_pin.length() + directs[i].to_pin.length()];
-        port_name = new char[directs[i].from_pin.length() + directs[i].to_pin.length()];
-
         // Load from pins
         // Parse out the pb_type name, port name, and pin range
-        parse_direct_pin_name(directs[i].from_pin.c_str(), directs[i].line, &start_pin_index, &end_pin_index, tile_name, port_name);
+        auto [start_pin_index, end_pin_index, tile_name, port_name] = parse_direct_pin_name(directs[i].from_pin, directs[i].line);
 
         // Figure out which type, port, and pin is used
-        for (const auto& type : device_ctx.physical_tile_types) {
-            if (strcmp(type.name, tile_name) == 0) {
+        for (const t_physical_tile_type& type : device_ctx.physical_tile_types) {
+            if (tile_name == type.name) {
                 physical_tile = &type;
                 break;
             }
         }
 
         if (physical_tile == nullptr) {
-            VPR_THROW(VPR_ERROR_ARCH, "Unable to find block %s.\n", tile_name);
+            VPR_THROW(VPR_ERROR_ARCH, "Unable to find block %s.\n", tile_name.c_str());
         }
 
         clb_to_clb_directs[i].from_clb_type = physical_tile;
@@ -4405,18 +4397,18 @@ static t_clb_to_clb_directs* alloc_and_load_clb_to_clb_directs(const std::vector
 
         // Load to pins
         // Parse out the pb_type name, port name, and pin range
-        parse_direct_pin_name(directs[i].to_pin.c_str(), directs[i].line, &start_pin_index, &end_pin_index, tile_name, port_name);
+        std::tie(start_pin_index, end_pin_index, tile_name, port_name) = parse_direct_pin_name(directs[i].to_pin, directs[i].line);
 
         // Figure out which type, port, and pin is used
-        for (const auto& type : device_ctx.physical_tile_types) {
-            if (strcmp(type.name, tile_name) == 0) {
+        for (const t_physical_tile_type& type : device_ctx.physical_tile_types) {
+            if (tile_name == type.name ) {
                 physical_tile = &type;
                 break;
             }
         }
 
         if (physical_tile == nullptr) {
-            VPR_THROW(VPR_ERROR_ARCH, "Unable to find block %s.\n", tile_name);
+            VPR_THROW(VPR_ERROR_ARCH, "Unable to find block %s.\n", tile_name.c_str());
         }
 
         clb_to_clb_directs[i].to_clb_type = physical_tile;
@@ -4447,8 +4439,6 @@ static t_clb_to_clb_directs* alloc_and_load_clb_to_clb_directs(const std::vector
             //Use the delayless switch by default
             clb_to_clb_directs[i].switch_index = delayless_switch;
         }
-        delete[] tile_name;
-        delete[] port_name;
     }
 
     return clb_to_clb_directs;
diff --git a/vpr/src/util/vpr_utils.cpp b/vpr/src/util/vpr_utils.cpp
@@ -1602,39 +1602,29 @@ void free_pb_stats(t_pb* pb) {
  * load placement macros in place_macro.c                                              *
  *                                                                                     *
  ***************************************************************************************/
-
-void parse_direct_pin_name(const char* src_string, int line, int* start_pin_index, int* end_pin_index, char* pb_type_name, char* port_name) {
-    /* Parses out the pb_type_name and port_name from the direct passed in.   *
-     * If the start_pin_index and end_pin_index is specified, parse them too. *
-     * Return the values parsed by reference.                                 */
-
-    char source_string[MAX_STRING_LEN + 1];
-    int ichar, match_count;
+std::tuple<int, int, std::string, std::string> parse_direct_pin_name(std::string_view src_string, int line) {
 
     if (vtr::split(src_string).size() > 1) {
         VPR_THROW(VPR_ERROR_ARCH,
                   "Only a single port pin range specification allowed for direct connect (was: '%s')", src_string);
     }
 
     // parse out the pb_type and port name, possibly pin_indices
-    const char* find_format = strstr(src_string, "[");
-    if (find_format == nullptr) {
+    if (src_string.find('[') == std::string_view::npos) {
         /* Format "pb_type_name.port_name" */
-        *start_pin_index = *end_pin_index = -1;
+        const int start_pin_index = -1;
+        const int end_pin_index = -1;
 
-        if (strlen(src_string) + 1 <= MAX_STRING_LEN + 1) {
-            strcpy(source_string, src_string);
-        } else {
-            VPR_FATAL_ERROR(VPR_ERROR_ARCH,
-                            "Pin name exceeded buffer size of %zu characters", MAX_STRING_LEN + 1);
-        }
-        for (ichar = 0; ichar < (int)(strlen(source_string)); ichar++) {
-            if (source_string[ichar] == '.')
-                source_string[ichar] = ' ';
-        }
+        std::string source_string{src_string};
+        // replace '.' characters with space
+        std::replace(source_string.begin(), source_string.end(), '.', ' ');
+
+        std::istringstream source_iss(source_string);
+        std::string pb_type_name, port_name;
 
-        match_count = sscanf(source_string, "%s %s", pb_type_name, port_name);
-        if (match_count != 2) {
+        if (source_iss >> pb_type_name >> port_name) {
+            return {start_pin_index, end_pin_index, pb_type_name, port_name};
+        } else {
             VTR_LOG_ERROR(
                 "[LINE %d] Invalid pin - %s, name should be in the format "
                 "\"pb_type_name\".\"port_name\" or \"pb_type_name\".\"port_name[end_pin_index:start_pin_index]\". "
@@ -1644,41 +1634,44 @@ void parse_direct_pin_name(const char* src_string, int line, int* start_pin_inde
         }
     } else {
         /* Format "pb_type_name.port_name[end_pin_index:start_pin_index]" */
-        strcpy(source_string, src_string);
-        for (ichar = 0; ichar < (int)(strlen(source_string)); ichar++) {
-            //Need white space between the components when using %s with
-            //sscanf
-            if (source_string[ichar] == '.')
-                source_string[ichar] = ' ';
-            if (source_string[ichar] == '[')
-                source_string[ichar] = ' ';
-        }
-
-        match_count = sscanf(source_string, "%s %s %d:%d]",
-                             pb_type_name, port_name,
-                             end_pin_index, start_pin_index);
-        if (match_count != 4) {
+        std::string source_string{src_string};
+
+        // Replace '.' and '[' characters with ' '
+        std::replace_if(source_string.begin(), source_string.end(),
+            [](char c) { return c == '.' || c == '['; },
+            ' ');
+
+        std::istringstream source_iss(source_string);
+        int start_pin_index, end_pin_index;
+        std::string pb_type_name, port_name;
+
+        if (source_iss >> pb_type_name >> port_name >> end_pin_index >> start_pin_index) {
+
+            if (end_pin_index < 0 || start_pin_index < 0) {
+                VTR_LOG_ERROR(
+                    "[LINE %d] Invalid pin - %s, the pin_index in "
+                    "[end_pin_index:start_pin_index] should not be a negative value.\n",
+                    line, src_string);
+                exit(1);
+            }
+
+            if (end_pin_index < start_pin_index) {
+                VTR_LOG_ERROR(
+                    "[LINE %d] Invalid from_pin - %s, the end_pin_index in "
+                    "[end_pin_index:start_pin_index] should not be less than start_pin_index.\n",
+                    line, src_string);
+                exit(1);
+            }
+
+            return {start_pin_index, end_pin_index, pb_type_name, port_name};
+        } else {
             VTR_LOG_ERROR(
                 "[LINE %d] Invalid pin - %s, name should be in the format "
                 "\"pb_type_name\".\"port_name\" or \"pb_type_name\".\"port_name[end_pin_index:start_pin_index]\". "
                 "The end_pin_index and start_pin_index can be the same.\n",
                 line, src_string);
             exit(1);
         }
-        if (*end_pin_index < 0 || *start_pin_index < 0) {
-            VTR_LOG_ERROR(
-                "[LINE %d] Invalid pin - %s, the pin_index in "
-                "[end_pin_index:start_pin_index] should not be a negative value.\n",
-                line, src_string);
-            exit(1);
-        }
-        if (*end_pin_index < *start_pin_index) {
-            VTR_LOG_ERROR(
-                "[LINE %d] Invalid from_pin - %s, the end_pin_index in "
-                "[end_pin_index:start_pin_index] should not be less than start_pin_index.\n",
-                line, src_string);
-            exit(1);
-        }
     }
 }
 
@@ -1824,10 +1817,10 @@ int get_atom_pin_class_num(const AtomPinId atom_pin_id) {
     return get_class_num_from_pin_physical_num(physical_type, pin_physical_num);
 }
 
-t_physical_tile_port find_tile_port_by_name(t_physical_tile_type_ptr type, const char* port_name) {
-    for (const auto& sub_tile : type->sub_tiles) {
-        for (const auto& port : sub_tile.ports) {
-            if (0 == strcmp(port.name, port_name)) {
+t_physical_tile_port find_tile_port_by_name(t_physical_tile_type_ptr type, std::string_view port_name) {
+    for (const t_sub_tile& sub_tile : type->sub_tiles) {
+        for (const t_physical_tile_port& port : sub_tile.ports) {
+            if (port_name == port.name) {
                 return port;
             }
         }
diff --git a/vpr/src/util/vpr_utils.h b/vpr/src/util/vpr_utils.h
@@ -216,7 +216,12 @@ t_pin_range get_pb_pins(t_physical_tile_type_ptr physical_type,
 float compute_primitive_base_cost(const t_pb_graph_node* primitive);
 int num_ext_inputs_atom_block(AtomBlockId blk_id);
 
-void parse_direct_pin_name(const char* src_string, int line, int* start_pin_index, int* end_pin_index, char* pb_type_name, char* port_name);
+/**
+ * @brief Parses out the pb_type_name and port_name from the direct passed in.
+ * If the start_pin_index and end_pin_index is specified, parse them too. *
+ * @return (start_pin_index, end_pin_index, pb_type_name, port_name)
+ */
+std::tuple<int, int, std::string, std::string> parse_direct_pin_name(std::string_view src_string, int line);
 
 void free_pb_stats(t_pb* pb);
 void free_pb(t_pb* pb);
