From: whitequark <whitequark@whitequark.org>
Date: Tue, 29 Jan 2019 02:24:00 +0000 (+0000)
Subject: write_verilog: correctly emit asynchronous transparent ports.
X-Git-Tag: yosys-0.9~316^2
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=da65e1e8d9552f64e1e03c08108ca0532719bbfe;p=yosys.git

write_verilog: correctly emit asynchronous transparent ports.

This commit fixes two related issues:
  * For asynchronous ports, clock is no longer added to domain list.
    (This would lead to absurd constructs like `always @(posedge 0)`.
  * The logic to distinguish synchronous and asynchronous ports is
    changed to correctly use or avoid clock in all cases.

Before this commit, the following RTLIL snippet (after memory_collect)

    cell $memrd $2
      parameter \MEMID "\\mem"
      parameter \ABITS 2
      parameter \WIDTH 4
      parameter \CLK_ENABLE 0
      parameter \CLK_POLARITY 1
      parameter \TRANSPARENT 1
      connect \CLK 1'0
      connect \EN 1'1
      connect \ADDR \mem_r_addr
      connect \DATA \mem_r_data
    end

would lead to invalid Verilog:

    reg [1:0] _0_;
    always @(posedge 1'h0) begin
      _0_ <= mem_r_addr;
    end
    assign mem_r_data = mem[_0_];

Note that there are two potential pitfalls remaining after this
change:
  * For asynchronous ports, the \EN input and \TRANSPARENT parameter
    are silently ignored. (Per discussion in #760 this is the correct
    behavior.)
  * For synchronous transparent ports, the \EN input is ignored. This
    matches the behavior of the $mem simulation cell. Again, see #760.
---

diff --git a/backends/verilog/verilog_backend.cc b/backends/verilog/verilog_backend.cc
index a7f329ef8..60668f1f0 100644
--- a/backends/verilog/verilog_backend.cc
+++ b/backends/verilog/verilog_backend.cc
@@ -1065,43 +1065,46 @@ bool dump_cell_expr(std::ostream &f, std::string indent, RTLIL::Cell *cell)
 			use_rd_clk = cell->parameters["\\RD_CLK_ENABLE"].extract(i).as_bool();
 			rd_clk_posedge = cell->parameters["\\RD_CLK_POLARITY"].extract(i).as_bool();
 			rd_transparent = cell->parameters["\\RD_TRANSPARENT"].extract(i).as_bool();
+			if (use_rd_clk)
 			{
-				std::ostringstream os;
-				dump_sigspec(os, sig_rd_clk);
-				clk_domain_str = stringf("%sedge %s", rd_clk_posedge ? "pos" : "neg", os.str().c_str());
-				if( clk_to_lof_body.count(clk_domain_str) == 0 )
-					clk_to_lof_body[clk_domain_str] = std::vector<std::string>();
-			}
-			if (use_rd_clk && !rd_transparent)
-			{
-				// for clocked read ports make something like:
-				//   reg [..] temp_id;
-				//   always @(posedge clk)
-				//      if (rd_en) temp_id <= array_reg[r_addr];
-				//   assign r_data = temp_id;
-				std::string temp_id = next_auto_id();
-				lof_reg_declarations.push_back( stringf("reg [%d:0] %s;\n", sig_rd_data.size() - 1, temp_id.c_str()) );
 				{
 					std::ostringstream os;
-					if (sig_rd_en != RTLIL::SigBit(true))
+					dump_sigspec(os, sig_rd_clk);
+					clk_domain_str = stringf("%sedge %s", rd_clk_posedge ? "pos" : "neg", os.str().c_str());
+					if( clk_to_lof_body.count(clk_domain_str) == 0 )
+						clk_to_lof_body[clk_domain_str] = std::vector<std::string>();
+				}
+				if (!rd_transparent)
+				{
+					// for clocked read ports make something like:
+					//   reg [..] temp_id;
+					//   always @(posedge clk)
+					//      if (rd_en) temp_id <= array_reg[r_addr];
+					//   assign r_data = temp_id;
+					std::string temp_id = next_auto_id();
+					lof_reg_declarations.push_back( stringf("reg [%d:0] %s;\n", sig_rd_data.size() - 1, temp_id.c_str()) );
+					{
+						std::ostringstream os;
+						if (sig_rd_en != RTLIL::SigBit(true))
+						{
+							os << stringf("if (");
+							dump_sigspec(os, sig_rd_en);
+							os << stringf(") ");
+						}
+						os << stringf("%s <= %s[", temp_id.c_str(), mem_id.c_str());
+						dump_sigspec(os, sig_rd_addr);
+						os << stringf("];\n");
+						clk_to_lof_body[clk_domain_str].push_back(os.str());
+					}
 					{
-						os << stringf("if (");
-						dump_sigspec(os, sig_rd_en);
-						os << stringf(") ");
+						std::ostringstream os;
+						dump_sigspec(os, sig_rd_data);
+						std::string line = stringf("assign %s = %s;\n", os.str().c_str(), temp_id.c_str());
+						clk_to_lof_body[""].push_back(line);
 					}
-					os << stringf("%s <= %s[", temp_id.c_str(), mem_id.c_str());
-					dump_sigspec(os, sig_rd_addr);
-					os << stringf("];\n");
-					clk_to_lof_body[clk_domain_str].push_back(os.str());
 				}
+				else
 				{
-					std::ostringstream os;
-					dump_sigspec(os, sig_rd_data);
-					std::string line = stringf("assign %s = %s;\n", os.str().c_str(), temp_id.c_str());
-					clk_to_lof_body[""].push_back(line);
-				}
-			} else {
-				if (rd_transparent) {
 					// for rd-transparent read-ports make something like:
 					//   reg [..] temp_id;
 					//   always @(posedge clk)
@@ -1121,15 +1124,15 @@ bool dump_cell_expr(std::ostream &f, std::string indent, RTLIL::Cell *cell)
 						std::string line = stringf("assign %s = %s[%s];\n", os.str().c_str(), mem_id.c_str(), temp_id.c_str());
 						clk_to_lof_body[""].push_back(line);
 					}
-				} else {
-					// for non-clocked read-ports make something like:
-					//   assign r_data = array_reg[r_addr];
-					std::ostringstream os, os2;
-					dump_sigspec(os, sig_rd_data);
-					dump_sigspec(os2, sig_rd_addr);
-					std::string line = stringf("assign %s = %s[%s];\n", os.str().c_str(), mem_id.c_str(), os2.str().c_str());
-					clk_to_lof_body[""].push_back(line);
 				}
+			} else {
+				// for non-clocked read-ports make something like:
+				//   assign r_data = array_reg[r_addr];
+				std::ostringstream os, os2;
+				dump_sigspec(os, sig_rd_data);
+				dump_sigspec(os2, sig_rd_addr);
+				std::string line = stringf("assign %s = %s[%s];\n", os.str().c_str(), mem_id.c_str(), os2.str().c_str());
+				clk_to_lof_body[""].push_back(line);
 			}
 		}