Skip to content

Commit e513dbb

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 0391d48 commit e513dbb

File tree

3 files changed

+59
-20
lines changed

3 files changed

+59
-20
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++) {

vpr/src/route/route_utils.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,7 @@
1919
#include "route_tree.h"
2020
#include "rr_graph.h"
2121
#include "tatum/TimingReporter.hpp"
22-
23-
#ifdef VPR_USE_TBB
2422
#include "stats.h"
25-
#endif // VPR_USE_TBB
2623

2724
bool check_net_delays(const Netlist<>& net_list, NetPinsMatrix<float>& net_delay) {
2825
constexpr float ERROR_TOL = 0.0001;

0 commit comments

Comments
 (0)