Skip to content

Commit 858e45f

Browse files
[FASM] Fixed Bug With Wire Creation
Found a bug within FASM's wire generation where it uses the index of the output pin to create the wire instead of the index of the input pin. This stemmed from some confusing code which both verified that the wire was being used and returning the first valid pin. It just so happens that it checked the outputs first and returned the output pin instead. Cleaned up the code and added more error checking to prevent issues like this in the future.
1 parent debf7c1 commit 858e45f

File tree

2 files changed

+59
-17
lines changed

2 files changed

+59
-17
lines changed

utils/fasm/src/fasm.cpp

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -217,26 +217,62 @@ std::string FasmWriterVisitor::build_clb_prefix(const t_pb *pb, const t_pb_graph
217217
return clb_prefix;
218218
}
219219

220-
static const t_pb_graph_pin* is_node_used(const t_pb_routes &top_pb_route, const t_pb_graph_node* pb_graph_node) {
221-
// Is the node used at all?
222-
const t_pb_graph_pin* pin = nullptr;
223-
for(int port_index = 0; port_index < pb_graph_node->num_output_ports; ++port_index) {
224-
for(int pin_index = 0; pin_index < pb_graph_node->num_output_pins[port_index]; ++pin_index) {
225-
pin = &pb_graph_node->output_pins[port_index][pin_index];
226-
if (top_pb_route.count(pin->pin_count_in_cluster) > 0 && top_pb_route[pin->pin_count_in_cluster].atom_net_id != AtomNetId::INVALID()) {
227-
return pin;
220+
/**
221+
* @brief Returns true if the given pin is used (i.e. is not "open").
222+
*/
223+
static bool is_pin_used(const t_pb_graph_pin* pin, const t_pb_routes &top_pb_route) {
224+
// A pin is used if it has a pb_route that is connected to an atom net.
225+
if (top_pb_route.count(pin->pin_count_in_cluster) == 0)
226+
return false;
227+
if (!top_pb_route[pin->pin_count_in_cluster].atom_net_id.is_valid())
228+
return false;
229+
return true;
230+
}
231+
232+
/**
233+
* @brief Returns the input pin for the given wire.
234+
*
235+
* Wires in VPR are a special primitive which is a LUT which acts like a wire
236+
* pass-through. Only one input of this LUT should be used.
237+
*/
238+
static const t_pb_graph_pin* get_wire_input_pin(const t_pb_routes &top_pb_route, const t_pb_graph_node* pb_graph_node) {
239+
const t_pb_graph_pin* wire_input_pin = nullptr;
240+
for(int port_index = 0; port_index < pb_graph_node->num_input_ports; ++port_index) {
241+
for(int pin_index = 0; pin_index < pb_graph_node->num_input_pins[port_index]; ++pin_index) {
242+
const t_pb_graph_pin* pin = &pb_graph_node->input_pins[port_index][pin_index];
243+
if (is_pin_used(pin, top_pb_route)) {
244+
VTR_ASSERT_MSG(wire_input_pin == nullptr,
245+
"Wire found with more than 1 used input");
246+
wire_input_pin = pin;
228247
}
229248
}
230249
}
231-
for(int port_index = 0; port_index < pb_graph_node->num_input_ports; ++port_index) {
232-
for(int pin_index = 0; pin_index < pb_graph_node->num_input_pins[port_index]; ++pin_index) {
233-
pin = &pb_graph_node->input_pins[port_index][pin_index];
234-
if (top_pb_route.count(pin->pin_count_in_cluster) > 0 && top_pb_route[pin->pin_count_in_cluster].atom_net_id != AtomNetId::INVALID()) {
235-
return pin;
250+
return wire_input_pin;
251+
}
252+
253+
/**
254+
* @brief Returns true if the given wire is used.
255+
*
256+
* A wire is used if it has a used output pin.
257+
*/
258+
static bool is_wire_used(const t_pb_routes &top_pb_route, const t_pb_graph_node* pb_graph_node) {
259+
// A wire is used if it has a used output pin.
260+
const t_pb_graph_pin* wire_output_pin = nullptr;
261+
for(int port_index = 0; port_index < pb_graph_node->num_output_ports; ++port_index) {
262+
for(int pin_index = 0; pin_index < pb_graph_node->num_output_pins[port_index]; ++pin_index) {
263+
const t_pb_graph_pin* pin = &pb_graph_node->output_pins[port_index][pin_index];
264+
if (is_pin_used(pin, top_pb_route)) {
265+
VTR_ASSERT_MSG(wire_output_pin == nullptr,
266+
"Wire found with more than 1 used output");
267+
wire_output_pin = pin;
236268
}
237269
}
238270
}
239-
return nullptr;
271+
272+
if (wire_output_pin != nullptr)
273+
return true;
274+
275+
return false;
240276
}
241277

242278
void FasmWriterVisitor::check_features(const t_metadata_dict *meta) const {
@@ -278,14 +314,19 @@ void FasmWriterVisitor::visit_all_impl(const t_pb_routes &pb_routes, const t_pb*
278314
}
279315

280316
if(mode != nullptr && std::string(mode->name) == "wire") {
281-
auto io_pin = is_node_used(pb_routes, pb_graph_node);
282-
if(io_pin != nullptr) {
283-
const auto& route = pb_routes.at(io_pin->pin_count_in_cluster);
317+
if (is_wire_used(pb_routes, pb_graph_node)) {
318+
const t_pb_graph_pin* wire_input_pin = get_wire_input_pin(pb_routes, pb_graph_node);
319+
VTR_ASSERT_MSG(wire_input_pin != nullptr,
320+
"Wire found with no used input pins");
321+
const auto& route = pb_routes.at(wire_input_pin->pin_count_in_cluster);
284322
const int num_inputs = *route.pb_graph_pin->parent_node->num_input_pins;
285323
const auto *lut_definition = find_lut(route.pb_graph_pin->parent_node);
286324
VTR_ASSERT(lut_definition->num_inputs == num_inputs);
287325

288326
output_fasm_features(lut_definition->CreateWire(route.pb_graph_pin->pin_number));
327+
} else {
328+
VTR_ASSERT_MSG(get_wire_input_pin(pb_routes, pb_graph_node) == nullptr,
329+
"Wire found with a used input pin, but no used output pin");
289330
}
290331
}
291332

vpr/src/base/read_netlist.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,7 @@ static void processPorts(pugi::xml_node Parent, t_pb* pb, t_pb_routes& pb_route,
755755
pb_route.insert(std::make_pair(rr_node_index, t_pb_route()));
756756
pb_route[rr_node_index].driver_pb_pin_id = pin_node[0][0]->pin_count_in_cluster;
757757
pb_route[rr_node_index].pb_graph_pin = pb_gpin;
758+
VTR_ASSERT(pb_route[rr_node_index].pb_graph_pin->pin_number == i);
758759

759760
found = false;
760761
for (j = 0; j < pin_node[0][0]->num_output_edges; j++) {

0 commit comments

Comments
 (0)