From 05026d50fc3129413622ad0d32c61b05e411004c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Zrounba?= Date: Tue, 14 Nov 2023 18:06:56 +0000 Subject: Fix bug in bidirectional devices handling In some cases (wrongly defined netlists), segmentation faults and wrong net handling could occur. It may need further checks in the near future. --- src/parser/parse_element.h | 12 +++++++----- src/parser/parse_tree.cpp | 29 +++++++++++++++++++---------- src/parser/parse_tree.h | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/parser/parse_element.h b/src/parser/parse_element.h index 58ad694..ebd08ab 100644 --- a/src/parser/parse_element.h +++ b/src/parser/parse_element.h @@ -40,11 +40,12 @@ using std::unique_ptr; virtual ParseElement *clone() const \ { return new ELEM_NAME(*this); } \ /* Implement virtual function create(...) to construct element */ \ - virtual sc_module *create(ParseTreeCreationHelper &pt_helper) const; \ + virtual sc_module *create(ParseTreeCreationHelper &pt_helper) const; \ virtual element_type_base * \ - instantiate_and_connect_uni(ParseTreeCreationHelper &pt_helper) const; \ + instantiate_and_connect_uni(ParseTreeCreationHelper &pt_helper) const; \ \ virtual string kind() const { return ELEM_NAME_LONG; } \ + virtual bool bidir_capable() const { return false; } \ }; /*************************************************/ @@ -64,13 +65,14 @@ using std::unique_ptr; virtual ParseElement *clone() const \ { return new ELEM_NAME(*this); } \ /* Implement virtual function create(...) to construct element */ \ - virtual sc_module *create(ParseTreeCreationHelper &pt_helper) const; \ + virtual sc_module *create(ParseTreeCreationHelper &pt_helper) const; \ virtual element_type_base * \ - instantiate_and_connect_uni(ParseTreeCreationHelper &pt_helper) const; \ + instantiate_and_connect_uni(ParseTreeCreationHelper &pt_helper) const; \ virtual element_type_base * \ - instantiate_and_connect_bi(ParseTreeCreationHelper &pt_helper) const; \ + instantiate_and_connect_bi(ParseTreeCreationHelper &pt_helper) const; \ \ virtual string kind() const { return string(ELEM_NAME_LONG) + " (bidir)"; } \ + virtual bool bidir_capable() const { return true; } \ }; /** ******************************************* **/ diff --git a/src/parser/parse_tree.cpp b/src/parser/parse_tree.cpp index bbafeb8..99dbcd3 100644 --- a/src/parser/parse_tree.cpp +++ b/src/parser/parse_tree.cpp @@ -29,7 +29,7 @@ vector> ParseNet::create(const string &name, bool force_bi else { assert(type() == OANALOG && "Only optical nets can be bidirectional for now"); - return { create_uni((name + "_0").c_str()), create_uni((name + "_1").c_str()) }; + return { create_uni((name + ":0").c_str()), create_uni((name + ":1").c_str()) }; } } @@ -181,10 +181,18 @@ void ParseTreeCreationHelper::upgrade_signal(const string &net_name) // The signal should exist because it has been created by pt_helper.create_signals() // but it doesn't hurt to check once more time assert(circuit_signals.find(net_name) != circuit_signals.end()); + assert(circuit_signals.at(net_name).size() >= 1); + assert(circuit_signals.at(net_name).size() <= 2); // Set the bidirectional flag to true pt->nets[net_name].m_bidirectional = true; + if (circuit_signals.at(net_name).size() == 2) + { + // No need to upgrade the signal (it's already bidirectional) + return; + } + // Check if the net was created as unidirectional (only one signal) if (circuit_signals.at(net_name).size() == 1) { @@ -196,10 +204,10 @@ void ParseTreeCreationHelper::upgrade_signal(const string &net_name) // if yes, that means a port is already connected to the signal // we cannot easily re-instantiate it as it may be bound to a port; // instead, we add a second signal to the vector to make it bidirectional - circuit_signals[net_name].push_back(pt->nets[net_name].create_uni(net_name + "_1")); + circuit_signals[net_name].push_back(pt->nets[net_name].create_uni(net_name + ":1")); - // For bidirectional signals, first port to connect binds to signal_0 - // for writing and signal_1 for reading + // For bidirectional signals, first port to connect binds to signal:0 + // for writing and signal:1 for reading // Therefore we have to "re-create" this scenario, as if the first device // connected to a bidirectional net. But for this we need to know which // if the first device was a writer or a reader. @@ -215,14 +223,14 @@ void ParseTreeCreationHelper::upgrade_signal(const string &net_name) if (has_writer) { // if a writer, then it's as expected: - // the next port will connect to signal_1 for writing + // the next port will connect to signal:1 for writing // we just need to update the number of "readers" connected to the net pt->nets[net_name].m_readers_count++; } else { // if a writer, then the next port needs to reverse its order. - // for this we swap signal_0 and signal_1 + // for this we swap signal:0 and signal:1 swap(circuit_signals[net_name][0], circuit_signals[net_name][1]); // then we need to update the number of "writers" connected to the net pt->nets[net_name].m_writers_count++; @@ -230,10 +238,11 @@ void ParseTreeCreationHelper::upgrade_signal(const string &net_name) } else { - // otherwise, we can recreate the first signal as well - circuit_signals[net_name].clear(); // will delete the net through the constructor + // otherwise, we can create the first signal as well + circuit_signals[net_name].clear(); // will delete the net through the destructor circuit_signals[net_name] = pt->nets.at(net_name).create(net_name); } + cout << "\t" << circuit_signals[net_name][0]->name() << "," << circuit_signals[net_name][1]->name() << endl; } } @@ -716,7 +725,7 @@ void ParseTree::build_circuit() { auto it = elements_backlog.begin(); auto &backlog_elem = *it; - cout << "Creating " << backlog_elem->name << " (reason: backlog)" << endl; + cout << "Creating - " << backlog_elem->name << " (reason: bidir backlog)" << endl; auto mod = backlog_elem->create(pt_helper); cout << "Done creating " << backlog_elem->name << endl; @@ -769,7 +778,7 @@ void ParseTree::build_circuit() { auto it = elements_backlog.begin(); auto &backlog_elem = *it; - cout << "Creating " << backlog_elem->name << " (reason: backlog)" << endl; + cout << "Creating " << backlog_elem->name << " (reason: unidir backlog)" << endl; auto mod = backlog_elem->create(pt_helper); cout << "Done creating " << backlog_elem->name << endl; diff --git a/src/parser/parse_tree.h b/src/parser/parse_tree.h index fbcefbc..54786de 100644 --- a/src/parser/parse_tree.h +++ b/src/parser/parse_tree.h @@ -560,6 +560,8 @@ struct ParseElement { } virtual string kind() const { return "UNDEFINED"; } + virtual bool bidir_capable() const { return false; } \ + virtual ~ParseElement() {} virtual string to_json() const; @@ -806,6 +808,7 @@ void ParseTreeCreationHelper::connect_uni(port_type &port, const string& net_nam const auto &net = pt->nets.at(net_name); // Get the signal to be written or read + sc_object *sig_raw; signal_type *sig = nullptr; if ( !net.bidirectional()) { @@ -814,7 +817,16 @@ void ParseTreeCreationHelper::connect_uni(port_type &port, const string& net_nam } else { - int i_write = pt->nets.at(net_name).m_connect_count; + // verify no more than 2 writer in total + if(pt->nets.at(net_name).m_connect_writer_count + as_writer > 2) + { + cerr << "Attempted to add a third writer to a bidirectional net (" + << net_name << ")." << endl; + exit(2); + } + + int i_write = pt->nets.at(net_name).m_connect_writer_count; + i_write = max(i_write, 1);// If there are already 2 writers, act as a reader for the second one (imitate the second writer but only as a reader) int i_read = (i_write + 1) % 2; int i = as_writer ? i_write : i_read; @@ -822,6 +834,14 @@ void ParseTreeCreationHelper::connect_uni(port_type &port, const string& net_nam sig = dynamic_cast(sig_raw); } + // Check corresponding signal could be found + if (!sig_raw) + { + cerr << "Missing object for signal connected to " + << net_name << endl; + exit(1); + } + // Check it could correctly be cast to requested type if (!sig) { @@ -849,13 +869,35 @@ void ParseTreeCreationHelper::connect_bi(port_in_type &p_in, port_out_type &p_ou if (net.bidirectional()) { + // verify no more than 2 writer in total + if(pt->nets.at(net_name).m_connect_writer_count >= 2) + { + cerr << "Attempted to add a third writer to a bidirectional net (" + << net_name << ")." << endl; + exit(1); + } // First get the signal to be written or read (in the right order) - int i_write = pt->nets.at(net_name).m_connect_count; + int i_write = pt->nets.at(net_name).m_connect_writer_count; int i_read = (i_write + 1) % 2; auto sig_writeable_raw = circuit_signals.at(net_name)[i_write].get(); auto sig_readable_raw = circuit_signals.at(net_name)[i_read].get(); + + // Check corresponding signals could be found + if (!sig_writeable_raw) + { + cerr << "Missing object for output signal connected to " + << net_name << endl; + exit(1); + } + if (!sig_readable_raw) + { + cerr << "Missing object for input signal connected to " + << net_name << endl; + exit(1); + } + auto sig_writeable = dynamic_cast(sig_writeable_raw); auto sig_readable = dynamic_cast(sig_readable_raw); -- cgit v1.2.3