From 73461971d2d68d9bd50bc1b06137d5b952d06b09 Mon Sep 17 00:00:00 2001 From: tangxifan Date: Sun, 28 Feb 2021 16:12:57 -0700 Subject: [PATCH] [Tool] Bug fix for printing single-bit ports in Verilog netlists --- .../libopenfpgautil/src/openfpga_port.cpp | 19 ++++++++++ .../libopenfpgautil/src/openfpga_port.h | 3 ++ .../verilog_formal_random_top_testbench.cpp | 3 ++ .../fpga_verilog/verilog_module_writer.cpp | 36 +++++++++++++------ .../verilog_preconfig_top_module.cpp | 3 ++ .../fpga_verilog/verilog_testbench_utils.cpp | 6 ++-- .../fpga_verilog/verilog_top_testbench.cpp | 3 ++ .../src/fpga_verilog/verilog_writer_utils.cpp | 25 ++++++++++--- .../src/fpga_verilog/verilog_writer_utils.h | 3 +- 9 files changed, 83 insertions(+), 18 deletions(-) diff --git a/libopenfpga/libopenfpgautil/src/openfpga_port.cpp b/libopenfpga/libopenfpgautil/src/openfpga_port.cpp index 023af7618..a68431739 100644 --- a/libopenfpga/libopenfpgautil/src/openfpga_port.cpp +++ b/libopenfpga/libopenfpgautil/src/openfpga_port.cpp @@ -22,27 +22,33 @@ BasicPort::BasicPort() { /* By default we set an invalid port, which size is 0 */ lsb_ = 1; msb_ = 0; + + origin_port_width_ = -1; } /* Quick constructor */ BasicPort::BasicPort(const char* name, const size_t& lsb, const size_t& msb) { set_name(std::string(name)); set_width(lsb, msb); + set_origin_port_width(-1); } BasicPort::BasicPort(const std::string& name, const size_t& lsb, const size_t& msb) { set_name(name); set_width(lsb, msb); + set_origin_port_width(-1); } BasicPort::BasicPort(const char* name, const size_t& width) { set_name(std::string(name)); set_width(width); + set_origin_port_width(-1); } BasicPort::BasicPort(const std::string& name, const size_t& width) { set_name(name); set_width(width); + set_origin_port_width(-1); } /* Copy constructor */ @@ -107,6 +113,11 @@ bool BasicPort::contained(const BasicPort& portA) const { return ( lsb_ <= portA.get_lsb() && portA.get_msb() <= msb_ ); } +/* Set original port width */ +size_t BasicPort::get_origin_port_width() const { + return origin_port_width_; +} + /************************************************************************ * Overloaded operators ***********************************************************************/ @@ -142,6 +153,7 @@ void BasicPort::set(const BasicPort& basic_port) { name_ = basic_port.get_name(); lsb_ = basic_port.get_lsb(); msb_ = basic_port.get_msb(); + origin_port_width_ = basic_port.get_origin_port_width(); return; } @@ -185,6 +197,11 @@ void BasicPort::set_msb(const size_t& msb) { return; } +void BasicPort::set_origin_port_width(const size_t& origin_port_width) { + origin_port_width_ = origin_port_width; + return; +} + /* Increase the port width */ void BasicPort::expand(const size_t& width) { if (0 == width) { @@ -291,6 +308,8 @@ void BasicPort::merge(const BasicPort& portA) { lsb_ = std::min((int)lsb_, (int)portA.get_lsb()); /* MSB follows the minium MSB of the two ports */ msb_ = std::max((int)msb_, (int)portA.get_msb()); + /* Origin port width follows the maximum of the two ports */ + msb_ = std::max((int)origin_port_width_, (int)portA.get_origin_port_width()); return; } diff --git a/libopenfpga/libopenfpgautil/src/openfpga_port.h b/libopenfpga/libopenfpgautil/src/openfpga_port.h index ca1afb4a5..4779e2332 100644 --- a/libopenfpga/libopenfpgautil/src/openfpga_port.h +++ b/libopenfpga/libopenfpgautil/src/openfpga_port.h @@ -31,6 +31,7 @@ class BasicPort { std::vector pins() const; /* Make a range of the pin indices */ bool mergeable(const BasicPort& portA) const; /* Check if a port can be merged with this port */ bool contained(const BasicPort& portA) const; /* Check if a port is contained by this port */ + size_t get_origin_port_width() const; public: /* Mutators */ void set(const BasicPort& basic_port); /* copy */ void set_name(const std::string& name); /* set the port LSB and MSB */ @@ -45,12 +46,14 @@ class BasicPort { void reset(); /* Reset to initial port */ void combine(const BasicPort& port); /* Combine two ports */ void merge(const BasicPort& portA); + void set_origin_port_width(const size_t& origin_port_width); private: /* internal functions */ void make_invalid(); /* Make a port invalid */ private: /* Internal Data */ std::string name_; /* Name of this port */ size_t msb_; /* Most Significant Bit of this port */ size_t lsb_; /* Least Significant Bit of this port */ + size_t origin_port_width_; /* Original port width of a port, used by traceback port conversion history */ }; /* Configuration ports: diff --git a/openfpga/src/fpga_verilog/verilog_formal_random_top_testbench.cpp b/openfpga/src/fpga_verilog/verilog_formal_random_top_testbench.cpp index 9c4ad47e5..860cc39a8 100644 --- a/openfpga/src/fpga_verilog/verilog_formal_random_top_testbench.cpp +++ b/openfpga/src/fpga_verilog/verilog_formal_random_top_testbench.cpp @@ -58,6 +58,9 @@ void print_verilog_top_random_testbench_ports(std::fstream& fp, const VprNetlistAnnotation& netlist_annotation) { /* Validate the file stream */ valid_file_stream(fp); + + print_verilog_default_net_type_declaration(fp, + VERILOG_DEFAULT_NET_TYPE_NONE); /* Print the declaration for the module */ fp << "module " << circuit_name << FORMAL_RANDOM_TOP_TESTBENCH_POSTFIX << ";" << std::endl; diff --git a/openfpga/src/fpga_verilog/verilog_module_writer.cpp b/openfpga/src/fpga_verilog/verilog_module_writer.cpp index 78d89c69e..18ae4af58 100644 --- a/openfpga/src/fpga_verilog/verilog_module_writer.cpp +++ b/openfpga/src/fpga_verilog/verilog_module_writer.cpp @@ -65,6 +65,7 @@ static BasicPort generate_verilog_port_for_module_net(const ModuleManager& module_manager, const ModuleId& module_id, const ModuleNetId& module_net) { + BasicPort port_to_return; /* Check all the sink modules of the net, * if we have a source module is the current module, this is not local wire */ @@ -73,7 +74,10 @@ BasicPort generate_verilog_port_for_module_net(const ModuleManager& module_manag /* Here, this is not a local wire, return the port name of the src_port */ ModulePortId net_src_port = module_manager.net_source_ports(module_id, module_net)[src_id]; size_t src_pin_index = module_manager.net_source_pins(module_id, module_net)[src_id]; - return BasicPort(module_manager.module_port(module_id, net_src_port).get_name(), src_pin_index, src_pin_index); + port_to_return.set(module_manager.module_port(module_id, net_src_port)); + port_to_return.set_width(src_pin_index, src_pin_index); + port_to_return.set_origin_port_width(module_manager.module_port(module_id, net_src_port).get_width()); + return port_to_return; } } @@ -83,7 +87,10 @@ BasicPort generate_verilog_port_for_module_net(const ModuleManager& module_manag /* Here, this is not a local wire, return the port name of the sink_port */ ModulePortId net_sink_port = module_manager.net_sink_ports(module_id, module_net)[sink_id]; size_t sink_pin_index = module_manager.net_sink_pins(module_id, module_net)[sink_id]; - return BasicPort(module_manager.module_port(module_id, net_sink_port).get_name(), sink_pin_index, sink_pin_index); + port_to_return.set(module_manager.module_port(module_id, net_sink_port)); + port_to_return.set_width(sink_pin_index, sink_pin_index); + port_to_return.set_origin_port_width(module_manager.module_port(module_id, net_sink_port).get_width()); + return port_to_return; } } @@ -110,8 +117,11 @@ BasicPort generate_verilog_port_for_module_net(const ModuleManager& module_manag net_name += std::string("_") + std::to_string(net_src_instance) + std::string("_"); net_name += module_manager.module_port(net_src_module, net_src_port).get_name(); } - - return BasicPort(net_name, net_src_pin, net_src_pin); + + port_to_return.set_name(net_name); + port_to_return.set_width(net_src_pin, net_src_pin); + port_to_return.set_origin_port_width(module_manager.module_port(net_src_module, net_src_port).get_width()); + return port_to_return; } /******************************************************************** @@ -422,6 +432,7 @@ void write_verilog_instance_to_file(std::fstream& fp, /* Create the port name and width to be used by the instance */ std::vector instance_ports; + std::vector instance_ports_is_single_bit; for (size_t child_pin : child_port.pins()) { /* Find the net linked to the pin */ ModuleNetId net = module_manager.module_instance_port_net(parent_module, child_module, instance_id, @@ -431,6 +442,7 @@ void write_verilog_instance_to_file(std::fstream& fp, /* We give the same port name as child module, this case happens to global ports */ instance_port.set_name(generate_verilog_undriven_local_wire_name(module_manager, parent_module, child_module, instance_id, child_port_id)); instance_port.set_width(child_pin, child_pin); + instance_port.set_origin_port_width(module_manager.module_port(child_module, child_port_id).get_width()); } else { /* Find the name for this child port */ instance_port = generate_verilog_port_for_module_net(module_manager, parent_module, net); @@ -478,13 +490,17 @@ void write_verilog_module_to_file(std::fstream& fp, /* Print an empty line as splitter */ fp << std::endl; - /* Print internal wires only when default net type is NOT wire */ - if (VERILOG_DEFAULT_NET_TYPE_WIRE != default_net_type) { - std::map> local_wires = find_verilog_module_local_wires(module_manager, module_id); - for (std::pair> port_group : local_wires) { - for (const BasicPort& local_wire : port_group.second) { - fp << generate_verilog_port(VERILOG_PORT_WIRE, local_wire) << ";" << std::endl; + /* Print internal wires */ + std::map> local_wires = find_verilog_module_local_wires(module_manager, module_id); + for (std::pair> port_group : local_wires) { + for (const BasicPort& local_wire : port_group.second) { + /* When default net type is wire, we can skip single-bit wires whose LSB is 0 */ + if ( (VERILOG_DEFAULT_NET_TYPE_WIRE == default_net_type) + && (1 == local_wire.get_width()) + && (0 == local_wire.get_lsb())) { + continue; } + fp << generate_verilog_port(VERILOG_PORT_WIRE, local_wire) << ";" << std::endl; } } diff --git a/openfpga/src/fpga_verilog/verilog_preconfig_top_module.cpp b/openfpga/src/fpga_verilog/verilog_preconfig_top_module.cpp index f963af6fe..542eadc2e 100644 --- a/openfpga/src/fpga_verilog/verilog_preconfig_top_module.cpp +++ b/openfpga/src/fpga_verilog/verilog_preconfig_top_module.cpp @@ -448,6 +448,9 @@ int print_verilog_preconfig_top_module(const ModuleManager &module_manager, std::string title = std::string("Verilog netlist for pre-configured FPGA fabric by design: ") + circuit_name; print_verilog_file_header(fp, title); + print_verilog_default_net_type_declaration(fp, + VERILOG_DEFAULT_NET_TYPE_NONE); + /* Print module declaration and ports */ print_verilog_preconfig_top_module_ports(fp, circuit_name, atom_ctx, netlist_annotation); diff --git a/openfpga/src/fpga_verilog/verilog_testbench_utils.cpp b/openfpga/src/fpga_verilog/verilog_testbench_utils.cpp index 7d0931905..b445e07f9 100644 --- a/openfpga/src/fpga_verilog/verilog_testbench_utils.cpp +++ b/openfpga/src/fpga_verilog/verilog_testbench_utils.cpp @@ -823,9 +823,10 @@ void rec_print_verilog_testbench_primitive_module_signal_initialization(std::fst /* Only for formal verification: deposite a zero signal values */ /* Initialize each input port */ BasicPort input_port_info(circuit_lib.port_lib_name(input_port), circuit_lib.port_size(input_port)); + input_port_info.set_origin_port_width(input_port_info.get_width()); fp << "\t\t$deposit("; fp << child_hie_path << "."; - fp << generate_verilog_port(VERILOG_PORT_CONKT, input_port_info); + fp << generate_verilog_port(VERILOG_PORT_CONKT, input_port_info, false); fp << ", " << circuit_lib.port_size(input_port) << "'b" << std::string(circuit_lib.port_size(input_port), '0'); fp << ");" << std::endl; } @@ -834,9 +835,10 @@ void rec_print_verilog_testbench_primitive_module_signal_initialization(std::fst /* Regular case: deposite initial signal values: a random value */ for (const auto& input_port : circuit_input_ports) { BasicPort input_port_info(circuit_lib.port_lib_name(input_port), circuit_lib.port_size(input_port)); + input_port_info.set_origin_port_width(input_port_info.get_width()); fp << "\t\t$deposit("; fp << child_hie_path << "."; - fp << generate_verilog_port(VERILOG_PORT_CONKT, input_port_info); + fp << generate_verilog_port(VERILOG_PORT_CONKT, input_port_info, false); fp << ", $random % 2 ? 1'b1 : 1'b0);" << std::endl; } diff --git a/openfpga/src/fpga_verilog/verilog_top_testbench.cpp b/openfpga/src/fpga_verilog/verilog_top_testbench.cpp index c45f3ce70..828779ec6 100644 --- a/openfpga/src/fpga_verilog/verilog_top_testbench.cpp +++ b/openfpga/src/fpga_verilog/verilog_top_testbench.cpp @@ -578,6 +578,9 @@ void print_verilog_top_testbench_ports(std::fstream& fp, /* Validate the file stream */ valid_file_stream(fp); + print_verilog_default_net_type_declaration(fp, + VERILOG_DEFAULT_NET_TYPE_NONE); + /* Print module definition */ fp << "module " << circuit_name << std::string(AUTOCHECK_TOP_TESTBENCH_VERILOG_MODULE_POSTFIX); fp << ";" << std::endl; diff --git a/openfpga/src/fpga_verilog/verilog_writer_utils.cpp b/openfpga/src/fpga_verilog/verilog_writer_utils.cpp index 15f801f7e..c4544a123 100644 --- a/openfpga/src/fpga_verilog/verilog_writer_utils.cpp +++ b/openfpga/src/fpga_verilog/verilog_writer_utils.cpp @@ -460,7 +460,8 @@ void print_verilog_module_end(std::fstream& fp, * Generate a string of a Verilog port ***********************************************/ std::string generate_verilog_port(const enum e_dump_verilog_port_type& verilog_port_type, - const BasicPort& port_info) { + const BasicPort& port_info, + const bool& must_print_port_size) { std::string verilog_line; /* Ensure the port type is valid */ @@ -472,8 +473,22 @@ std::string generate_verilog_port(const enum e_dump_verilog_port_type& verilog_p * others require a format of [:] */ if (VERILOG_PORT_CONKT == verilog_port_type) { - /* When LSB == MSB, we can use a simplified format []*/ - if ( 1 == port_info.get_width()) { + /* Simplication: + * - When LSB == MSB == 0, we do not need to specify size when the user option allows + * Note that user option is essential, otherwise what could happen is that + * a multi-bit verilog port used in instance port mapping is printed as a single-bit net + * For example, + * input [1:0] in; + * inv inv_inst (.A(in), .Y(out)); + * The original port width is the reference to backtrace the defintion of the port + * - When LSB == MSB, we can use a simplified format [] + */ + if ((false == must_print_port_size) + && (1 == port_info.get_width()) + && (0 == port_info.get_lsb()) + && (1 == port_info.get_origin_port_width())) { + size_str.clear(); + } else if ((1 == port_info.get_width()) && (0 != port_info.get_lsb())) { size_str = "[" + std::to_string(port_info.get_lsb()) + "]"; } verilog_line = port_info.get_name() + size_str; @@ -582,7 +597,7 @@ std::string generate_verilog_ports(const std::vector& merged_ports) { VTR_ASSERT(0 < merged_ports.size()); if ( 1 == merged_ports.size()) { /* Use connection type of verilog port */ - return generate_verilog_port(VERILOG_PORT_CONKT, merged_ports[0]); + return generate_verilog_port(VERILOG_PORT_CONKT, merged_ports[0], false); } std::string verilog_line = "{"; @@ -591,7 +606,7 @@ std::string generate_verilog_ports(const std::vector& merged_ports) { if (&port != &merged_ports[0]) { verilog_line += ", "; } - verilog_line += generate_verilog_port(VERILOG_PORT_CONKT, port); + verilog_line += generate_verilog_port(VERILOG_PORT_CONKT, port, false); } verilog_line += "}"; diff --git a/openfpga/src/fpga_verilog/verilog_writer_utils.h b/openfpga/src/fpga_verilog/verilog_writer_utils.h index 59c05aea9..61f13385e 100644 --- a/openfpga/src/fpga_verilog/verilog_writer_utils.h +++ b/openfpga/src/fpga_verilog/verilog_writer_utils.h @@ -85,7 +85,8 @@ void print_verilog_module_end(std::fstream& fp, const std::string& module_name); std::string generate_verilog_port(const enum e_dump_verilog_port_type& dump_port_type, - const BasicPort& port_info); + const BasicPort& port_info, + const bool& must_print_port_size = true); bool two_verilog_ports_mergeable(const BasicPort& portA, const BasicPort& portB);