Fix handling of combined pin/segment <fc_overrides>

Previously these were not interpreted correctly leading to incorrect
behaviour (and seg faults) on some complex switch block architectures.

Also update rr graph Fc calculation to ensure Fc_in is only calculated for
input pins, and Fc_out only for output pins.

Author: kmurray

libarchfpga/src/echo_arch.cpp

@@ -78,10 +78,10 @@ void EchoArch(const char *EchoFile, const t_type_descriptor* Types,
 		fprintf(Echo, "\twidth: %d\n", Types[i].width);
 		fprintf(Echo, "\theight: %d\n", Types[i].height);
 		for (const t_fc_specification& fc_spec : Types[i].fc_specs) {
-            fprintf(Echo, "fc_type: ");
-            if (fc_spec.fc_type == e_fc_type::ABSOLUTE) {
+            fprintf(Echo, "fc_value_type: ");
+            if (fc_spec.fc_value_type == e_fc_value_type::ABSOLUTE) {
                 fprintf(Echo, "ABSOLUTE");
-            } else if (fc_spec.fc_type == e_fc_type::FRACTIONAL) {
+            } else if (fc_spec.fc_value_type == e_fc_value_type::FRACTIONAL) {
                 fprintf(Echo, "FRACTIONAL");
             } else {
                 VTR_ASSERT(false);

libarchfpga/src/physical_types.h

@@ -270,16 +270,23 @@ struct s_port_power {
 
 //The type of Fc specification
 enum class e_fc_type {
+    IN, //The fc specification for an input pin
+    OUT //The fc specification for an output pin
+};
+
+//The value type of the Fc specification
+enum class e_fc_value_type {
     FRACTIONAL,   //Fractional Fc specification (i.e. fraction of routing channel tracks)
-    ABSOLUTE     //Absolute Fc specification (i.e. absolute number of tracks)
+    ABSOLUTE      //Absolute Fc specification (i.e. absolute number of tracks)
 };
 
 //Describes the Fc specification for a set of pins and a segment
 struct t_fc_specification {
-    e_fc_type fc_type;      //How to interpret the Fc value
-    float fc_value;         //The Fc value
-    int seg_index;          //The target segment index
-    std::vector<int> pins;  //The block pins collectively effected by this Fc
+    e_fc_type fc_type;              //What type of Fc
+    e_fc_value_type fc_value_type;  //How to interpret the Fc value
+    float fc_value;                 //The Fc value
+    int seg_index;                  //The target segment index
+    std::vector<int> pins;          //The block pins collectively effected by this Fc
 };
 
 /* Describes the type for a physical logic block

libarchfpga/src/read_xml_arch_file.cpp

@@ -66,7 +66,7 @@ using namespace pugiutil;
 struct t_fc_override {
     std::string port_name;
     std::string seg_name;
-    e_fc_type fc_type;
+    e_fc_value_type fc_value_type;
     float fc_value;
 };
 
@@ -103,7 +103,7 @@ static void ProcessMode(pugi::xml_node Parent, t_mode * mode, const t_arch& arch
 		const pugiutil::loc_data& loc_data);
 static void Process_Fc(pugi::xml_node Node, t_type_descriptor * Type, t_segment_inf *segments, int num_segments, const pugiutil::loc_data& loc_data);
 static t_fc_override Process_Fc_override(pugi::xml_node node, const pugiutil::loc_data& loc_data);
-static e_fc_type string_to_fc_type(const std::string& str, pugi::xml_node node, const pugiutil::loc_data& loc_data);
+static e_fc_value_type string_to_fc_value_type(const std::string& str, pugi::xml_node node, const pugiutil::loc_data& loc_data);
 static void ProcessComplexBlockProps(pugi::xml_node Node, t_type_descriptor * Type, const pugiutil::loc_data& loc_data);
 static void ProcessSizingTimingIpinCblock(pugi::xml_node Node,
 		struct s_arch *arch, const bool timing_enabled, const pugiutil::loc_data& loc_data);
@@ -1626,21 +1626,21 @@ static void ProcessMode(pugi::xml_node Parent, t_mode * mode, const t_arch& arch
 /* Takes in the node ptr for the 'fc' elements and initializes
  * the appropriate fields of type. */
 static void Process_Fc(pugi::xml_node Node, t_type_descriptor * Type, t_segment_inf *segments, int num_segments, const pugiutil::loc_data& loc_data) {
-    e_fc_type default_fc_in_type = e_fc_type::FRACTIONAL;
-    e_fc_type default_fc_out_type = e_fc_type::FRACTIONAL;
+    e_fc_value_type default_fc_in_value_type = e_fc_value_type::FRACTIONAL;
+    e_fc_value_type default_fc_out_value_type = e_fc_value_type::FRACTIONAL;
     float default_fc_in_value = std::numeric_limits<float>::quiet_NaN();
     float default_fc_out_value = std::numeric_limits<float>::quiet_NaN();
 
 	/* Load the default fc_in */
 	auto default_fc_in_attrib = get_attribute(Node, "default_in_type", loc_data);
-    default_fc_in_type = string_to_fc_type(default_fc_in_attrib.value(), Node, loc_data);
+    default_fc_in_value_type = string_to_fc_value_type(default_fc_in_attrib.value(), Node, loc_data);
 
     auto default_in_val_attrib = get_attribute(Node, "default_in_val", loc_data);
     default_fc_in_value = vtr::atof(default_in_val_attrib.value());
 
 	/* Load the default fc_out */
 	auto default_fc_out_attrib = get_attribute(Node, "default_out_type", loc_data);
-    default_fc_out_type = string_to_fc_type(default_fc_out_attrib.value(), Node, loc_data);
+    default_fc_out_value_type = string_to_fc_value_type(default_fc_out_attrib.value(), Node, loc_data);
 
     auto default_out_val_attrib = get_attribute(Node, "default_out_val", loc_data);
     default_fc_out_value = vtr::atof(default_out_val_attrib.value());
@@ -1669,25 +1669,60 @@ static void Process_Fc(pugi::xml_node Node, t_type_descriptor * Type, t_segment_
                 const t_port* port = &pb_type->ports[iport];
 
                 t_fc_specification fc_spec;
+
                 fc_spec.seg_index = iseg;
 
-                //Apply the defaults
+                //Apply type and defaults
                 if (port->type == IN_PORT) {
-                    fc_spec.fc_type = default_fc_in_type;
+                    fc_spec.fc_type = e_fc_type::IN;
+                    fc_spec.fc_value_type = default_fc_in_value_type;
                     fc_spec.fc_value = default_fc_in_value;
                 } else {
                     VTR_ASSERT(port->type == OUT_PORT);
-                    fc_spec.fc_type = default_fc_out_type;
+                    fc_spec.fc_type = e_fc_type::OUT;
+                    fc_spec.fc_value_type = default_fc_out_value_type;
                     fc_spec.fc_value = default_fc_out_value;
                 }
 
                 //Apply any matching overrides
+                bool default_overriden = false;
                 for(const auto& fc_override : fc_overrides) {
-                    if (fc_override.port_name == port->name || fc_override.seg_name == segments[iseg].name) {
+                    bool apply_override = false;
+                    if (!fc_override.port_name.empty() && !fc_override.seg_name.empty()) {
+                        //Both port and seg names are specified require exact match on both
+                        if (fc_override.port_name == port->name && fc_override.seg_name == segments[iseg].name) {
+                            apply_override = true;
+                        }
+                        
+                    } else if (!fc_override.port_name.empty()) {
+                        VTR_ASSERT(fc_override.seg_name.empty());
+                        //Only the port name specified, require it to match
+                        if (fc_override.port_name == port->name) {
+                            apply_override = true;
+                        }
+                    } else {
+                        VTR_ASSERT(!fc_override.seg_name.empty());
+                        VTR_ASSERT(fc_override.port_name.empty());
+                        //Only the seg name specified, require it to match
+                        if (fc_override.seg_name == segments[iseg].name) {
+                            apply_override = true;
+                        }
+                    }
+
+                    if (apply_override) {
                         //Exact match, or partial match to either port or seg name
-                        // Note that we continue searching, this ensures that the last matching override is applied last
-                        fc_spec.fc_type = fc_override.fc_type;
+                        // Note that we continue searching, this ensures that the last matching override (in file order)
+                        // is applied last
+
+                        if (default_overriden) {
+                            //Warn if multiple overrides match
+                            vtr::printf_warning(loc_data.filename_c_str(), loc_data.line(Node), "Multiple matching Fc overrides found; the last will be applied\n");
+                        }
+
+                        fc_spec.fc_value_type = fc_override.fc_value_type;
                         fc_spec.fc_value = fc_override.fc_value;
+
+                        default_overriden = true;
                     }
                 }
 
@@ -1727,7 +1762,7 @@ static t_fc_override Process_Fc_override(pugi::xml_node node, const pugiutil::lo
             fc_override.seg_name = attrib.value();
             seen_port_or_seg |= true;
         } else if (attrib.name() == std::string("fc_type")) {
-            fc_override.fc_type = string_to_fc_type(attrib.value(), node, loc_data);
+            fc_override.fc_value_type = string_to_fc_value_type(attrib.value(), node, loc_data);
             seen_fc_type = true;
         } else if (attrib.name() == std::string("fc_val")) {
             fc_override.fc_value = vtr::atof(attrib.value());
@@ -1756,20 +1791,20 @@ static t_fc_override Process_Fc_override(pugi::xml_node node, const pugiutil::lo
     return fc_override;
 }
 
-static e_fc_type string_to_fc_type(const std::string& str, pugi::xml_node node, const pugiutil::loc_data& loc_data) {
-    e_fc_type fc_type = e_fc_type::FRACTIONAL;
+static e_fc_value_type string_to_fc_value_type(const std::string& str, pugi::xml_node node, const pugiutil::loc_data& loc_data) {
+    e_fc_value_type fc_value_type = e_fc_value_type::FRACTIONAL;
 
     if (str == "frac") {
-        fc_type = e_fc_type::FRACTIONAL;
+        fc_value_type = e_fc_value_type::FRACTIONAL;
     } else if (str == "abs") {
-        fc_type = e_fc_type::ABSOLUTE;
+        fc_value_type = e_fc_value_type::ABSOLUTE;
     } else {
         archfpga_throw(loc_data.filename_c_str(), loc_data.line(node),
                 "Invalid fc_type '%s'. Must be 'abs' or 'frac'.\n",
                 str.c_str());
     }
 
-    return fc_type;
+    return fc_value_type;
 }
 
 /* Thie processes attributes of the 'type' tag */

vpr/SRC/base/vpr_api.cpp

@@ -526,7 +526,7 @@ static void get_intercluster_switch_fanin_estimates(const t_vpr_setup& vpr_setup
     for (const t_fc_specification& fc_spec : FILL_TYPE->fc_specs) {
         float Fc = fc_spec.fc_value;
 
-        if (fc_spec.fc_type == e_fc_type::ABSOLUTE) {
+        if (fc_spec.fc_value_type == e_fc_value_type::ABSOLUTE) {
             //Convert to estimated fractional
             Fc /= W;
         }

vpr/SRC/route/rr_graph.cpp

@@ -213,7 +213,7 @@ static t_seg_details *alloc_and_load_global_route_seg_details(
 
 static int ***alloc_and_load_actual_fc(const int L_num_types, const t_type_ptr types, const int max_pins,
 		const int num_seg_types, const int *sets_per_seg_type,
-		const int max_chan_width,
+		const int max_chan_width, const e_fc_type fc_type,
 		const enum e_directionality directionality, 
 		bool *Fc_clipped, const bool ignore_Fc_0);
 
@@ -340,27 +340,29 @@ void build_rr_graph(
 	} else {
 		bool Fc_clipped = false;
 		Fc_in = alloc_and_load_actual_fc(L_num_types, types, max_pins, num_seg_types, sets_per_seg_type, max_chan_width,
-				directionality, &Fc_clipped, ignore_Fc_0);
+				e_fc_type::IN, directionality, &Fc_clipped, ignore_Fc_0);
 		if (Fc_clipped) {
 			*Warnings |= RR_GRAPH_WARN_FC_CLIPPED;
 		}
 		Fc_clipped = false;
 		Fc_out = alloc_and_load_actual_fc(L_num_types, types, max_pins, num_seg_types, sets_per_seg_type, max_chan_width,
-				directionality, &Fc_clipped, ignore_Fc_0);
+				e_fc_type::OUT, directionality, &Fc_clipped, ignore_Fc_0);
 		if (Fc_clipped) {
 			*Warnings |= RR_GRAPH_WARN_FC_CLIPPED;
 		}
 
-#ifdef VERBOSE
 		for (int i = 1; i < L_num_types; ++i) { /* Skip "<EMPTY>" */
 			for (int j = 0; j < g_block_types[i].num_pins; ++j) {
 				for (int k = 0; k < num_seg_types; k++){
+#ifdef VERBOSE
                     vtr::printf_info("Fc Actual Values: type = %s, pin = %d, seg = %d (%s), Fc_out = %d, Fc_in = %d.\n",
                             g_block_types[i].name, j, k, segment_inf[k].name, Fc_out[i][j][k], Fc_in[i][j][k]);
+#endif /* VERBOSE */
+                    VTR_ASSERT_MSG(Fc_out[i][j][k] == 0 || Fc_in[i][j][k] == 0, 
+                                  "Pins must be inputs or outputs (i.e. can not have both non-zero Fc_out and Fc_in)");
 				}
 			}
 		}
-#endif /* VERBOSE */
 	}
 
 	bool **perturb_ipins = alloc_and_load_perturb_ipins(L_num_types, num_seg_types,
@@ -890,7 +892,7 @@ static t_seg_details *alloc_and_load_global_route_seg_details(
 /* Calculates the number of track connections from each block pin to each segment type */
 static int ***alloc_and_load_actual_fc(const int L_num_types, const t_type_ptr types, const int max_pins,
 		const int num_seg_types, const int *sets_per_seg_type,
-		const int max_chan_width,
+		const int max_chan_width, const e_fc_type fc_type,
 		const enum e_directionality directionality, 
 		bool *Fc_clipped, const bool ignore_Fc_0) {
 
@@ -920,6 +922,8 @@ static int ***alloc_and_load_actual_fc(const int L_num_types, const t_type_ptr t
 	for (int itype = 1; itype < L_num_types; ++itype) { //Skip <EMPTY>
         for (const t_fc_specification fc_spec : types[itype].fc_specs) {
 
+            if (fc_type != fc_spec.fc_type) continue;
+
             int iseg = fc_spec.seg_index;
 
             if(fc_spec.fc_value == 0 && ignore_Fc_0 == false) {
@@ -932,11 +936,11 @@ static int ***alloc_and_load_actual_fc(const int L_num_types, const t_type_ptr t
 
                 //Calculate how many connections there should be accross all the pins in this fc_spec
                 float flt_total_connections;
-                if (fc_spec.fc_type == e_fc_type::FRACTIONAL) {
+                if (fc_spec.fc_value_type == e_fc_value_type::FRACTIONAL) {
                     float conns_per_pin = fac * sets_per_seg_type[iseg] * fc_spec.fc_value;
                     flt_total_connections = conns_per_pin * fc_spec.pins.size();
                 } else {
-                    VTR_ASSERT(fc_spec.fc_type == e_fc_type::ABSOLUTE);
+                    VTR_ASSERT(fc_spec.fc_value_type == e_fc_value_type::ABSOLUTE);
                     flt_total_connections = fc_spec.fc_value * fc_spec.pins.size();
                 }
 
@@ -2188,6 +2192,7 @@ static vtr::t_ivec ****alloc_and_load_track_to_pin_lookup(
 
 					for (int conn = 0; conn < num_tracks; ++conn) {
 						int track = pin_to_track_map[pin][width][height][side][conn];
+                        VTR_ASSERT(track < max_chan_width);
 						track_to_pin_lookup[track][width][height][side].nelem++;
 					}
 				}