Merge pull request #386 from pllab/verilog-warning-on-reset

Add add_reset to verilog test bench generation, and add check for existing `rst` wire.

Author: mdko

pyrtl/importexport.py

@@ -432,6 +432,11 @@ def output_to_verilog(dest_file, add_reset=True, block=None):
     file = dest_file
     internal_names = _VerilogSanitizer('_ver_out_tmp_')
 
+    if add_reset:  # True or 'asynchronous'
+        if block.get_wirevector_by_name('rst') is not None:
+            raise PyrtlError("Found a user-defined wire named 'rst'. Pass in "
+                             "'add_reset=False' to use your existing reset logic.")
+
     for wire in block.wirevector_set:
         internal_names.make_valid_string(wire.name)
 
@@ -672,7 +677,7 @@ def _to_verilog_footer(file):
 
 
 def output_verilog_testbench(dest_file, simulation_trace=None, toplevel_include=None,
-                             vcd="waveform.vcd", cmd=None, block=None):
+                             vcd="waveform.vcd", cmd=None, add_reset=True, block=None):
     """ Output a Verilog testbench for the block/inputs used in the simulation trace.
 
     :param dest_file: an open file to which the test bench will be printed.
@@ -692,6 +697,16 @@ def output_verilog_testbench(dest_file, simulation_trace=None, toplevel_include=
         specific values out during testbench evaluation (e.g. cmd='$display("%d", out);'
         will instruct the testbench to print the value of 'out' every cycle which can then
         be compared easy with a reference).
+    :param add_reset: If reset logic should be added. Allowable options are:
+        False (meaning no reset logic is added), True (default, for adding synchronous
+        reset logic), and 'asynchronous' (for adding asynchronous reset logic).
+        The value passed in here should match the argument passed to `output_to_verilog()`.
+    :param block: Block containing design to test.
+
+    If `add_reset` is not False, a `rst` wire is added and will passed as an input to the
+    instantiated toplevel module. The `rst` wire will be held low in the testbench, because
+    initialization here occurs via the `initial` block. It is provided for consistency with
+    `output_to_verilog()`.
 
     The test bench does not return any values.
 
@@ -707,8 +722,18 @@ def output_verilog_testbench(dest_file, simulation_trace=None, toplevel_include=
             output_verilog_testbench(fp, sim.tracer, vcd=None, cmd='$display("%d", out);')
 
     """
+    if not isinstance(add_reset, bool):
+        if add_reset != 'asynchronous':
+            raise PyrtlError("Invalid add_reset option %s. Acceptable options are "
+                             "False, True, and 'asynchronous'")
+
     block = working_block(block)
 
+    if add_reset:  # True or 'asynchronous'
+        if block.get_wirevector_by_name('rst') is not None:
+            raise PyrtlError("Found a user-defined wire named 'rst'. Pass in "
+                             "'add_reset=False' to use your existing reset logic.")
+
     inputs, outputs, registers, wires, memories = _verilog_block_parts(block)
 
     ver_name = _VerilogSanitizer('_ver_out_tmp_')
@@ -760,6 +785,8 @@ def default_value():
 
     # Declare all block inputs as reg
     print('    reg clk;', file=dest_file)
+    if add_reset:
+        print('    reg rst;', file=dest_file)
     for w in name_sorted(inputs):
         print('    reg{:s} {:s};'.format(_verilog_vector_decl(w), ver_name[w.name]),
               file=dest_file)
@@ -775,6 +802,8 @@ def default_value():
 
     # Instantiate logic block
     io_list = ['clk'] + name_list(name_sorted(inputs)) + name_list(name_sorted(outputs))
+    if add_reset:
+        io_list.insert(1, 'rst')
     io_list_str = ['.{0:s}({0:s})'.format(w) for w in io_list]
     print('    toplevel block({:s});\n'.format(', '.join(io_list_str)), file=dest_file)
 
@@ -792,6 +821,8 @@ def default_value():
 
     # Initialize clk, and all the registers and memories
     print('        clk = 0;', file=dest_file)
+    if add_reset:
+        print('        rst = 0;', file=dest_file)
     for r in name_sorted(registers):
         print('        block.%s = %d;' % (ver_name[r.name], init_regvalue(r)), file=dest_file)
     for m in sorted(memories, key=lambda m: m.id):

tests/test_importexport.py

@@ -1281,55 +1281,52 @@ def test_blif_with_clock_passing(self):
 """
 
 
-verilog_test_bench = """\
-module tb();
-    reg clk;
-    reg[1:0] a100;
-    reg[3:0] w1;
-    reg[2:0] w12;
-    wire[1:0] out1;
-    wire[8:0] out10;
-
-    integer tb_iter;
-    toplevel block(.clk(clk), .a100(a100), .w1(w1), .w12(w12), .out1(out1), .out10(out10));
-
-    always
-        #5 clk = ~clk;
-
-    initial begin
-        $dumpfile ("waveform.vcd");
-        $dumpvars;
+verilog_custom_reset = """\
+// Generated automatically via PyRTL
+// As one initial test of synthesis, map to FPGA with:
+//   yosys -p "synth_xilinx -top toplevel" thisfile.v
 
-        clk = 0;
-        block.r1 = 2;
-        block.r2 = 3;
-        block.tmp13 = 0;
-        for (tb_iter = 0; tb_iter < 32; tb_iter++) begin block.mem_0[tb_iter] = 0; end
-        block.mem_0[2] = 9;
-        block.mem_0[9] = 12;
-        a100 = 2'd0;
-        w1 = 4'd0;
-        w12 = 3'd0;
+module toplevel(clk, rst);
+    input clk;
+    input rst;
 
-        #10
-        a100 = 2'd1;
-        w1 = 4'd4;
-        w12 = 3'd1;
+    reg[3:0] r;
 
-        #10
-        a100 = 2'd3;
-        w1 = 4'd2;
-        w12 = 3'd7;
+    wire const_0_1;
+    wire const_1_0;
+    wire const_2_0;
+    wire const_3_0;
+    wire[2:0] tmp0;
+    wire[3:0] tmp1;
+    wire[4:0] tmp2;
+    wire[3:0] tmp3;
+    wire[4:0] tmp4;
+    wire[4:0] tmp5;
+    wire[3:0] tmp6;
 
-        #10
-        a100 = 2'd2;
-        w1 = 4'd3;
-        w12 = 3'd4;
+    // Combinational
+    assign const_0_1 = 1;
+    assign const_1_0 = 0;
+    assign const_2_0 = 0;
+    assign const_3_0 = 0;
+    assign tmp0 = {const_1_0, const_1_0, const_1_0};
+    assign tmp1 = {tmp0, const_0_1};
+    assign tmp2 = r + tmp1;
+    assign tmp3 = {const_3_0, const_3_0, const_3_0, const_3_0};
+    assign tmp4 = {tmp3, const_2_0};
+    assign tmp5 = rst ? tmp4 : tmp2;
+    assign tmp6 = {tmp5[3], tmp5[2], tmp5[1], tmp5[0]};
 
-        #10
-        $finish;
+    // Registers
+    always @(posedge clk)
+    begin
+        begin
+            r <= tmp6;
+        end
     end
+
 endmodule
+
 """
 
 
@@ -1452,10 +1449,156 @@ def test_error_invalid_add_reset(self):
         with self.assertRaisesRegex(pyrtl.PyrtlError, "Invalid add_reset option"):
             pyrtl.output_to_verilog(buffer, add_reset='foobar')
 
+    def test_error_existing_reset_wire(self):
+        buffer = io.StringIO()
+        _rst = pyrtl.Input(1, 'rst')
+        with self.assertRaisesRegex(pyrtl.PyrtlError, "Found a user-defined wire named 'rst'."):
+            pyrtl.output_to_verilog(buffer)
+
+    def test_existing_reset_wire_without_add_reset(self):
+        buffer = io.StringIO()
+        rst = pyrtl.Input(1, 'rst')
+        r = pyrtl.Register(4, 'r')
+        r.next <<= pyrtl.select(rst, 0, r + 1)
+        pyrtl.output_to_verilog(buffer, add_reset=False)
+        self.assertEqual(buffer.getvalue(), verilog_custom_reset)
+
+
+verilog_testbench = """\
+module tb();
+    reg clk;
+    reg rst;
+    reg[1:0] a100;
+    reg[3:0] w1;
+    reg[2:0] w12;
+    wire[1:0] out1;
+    wire[8:0] out10;
+
+    integer tb_iter;
+    toplevel block(.clk(clk), .rst(rst), .a100(a100), .w1(w1), .w12(w12), .out1(out1), .out10(out10));
+
+    always
+        #5 clk = ~clk;
+
+    initial begin
+        $dumpfile ("waveform.vcd");
+        $dumpvars;
+
+        clk = 0;
+        rst = 0;
+        block.r1 = 2;
+        block.r2 = 3;
+        block.tmp0 = 0;
+        for (tb_iter = 0; tb_iter < 32; tb_iter++) begin block.mem_0[tb_iter] = 0; end
+        block.mem_0[2] = 9;
+        block.mem_0[9] = 12;
+        a100 = 2'd0;
+        w1 = 4'd0;
+        w12 = 3'd0;
+
+        #10
+        a100 = 2'd1;
+        w1 = 4'd4;
+        w12 = 3'd1;
+
+        #10
+        a100 = 2'd3;
+        w1 = 4'd2;
+        w12 = 3'd7;
+
+        #10
+        a100 = 2'd2;
+        w1 = 4'd3;
+        w12 = 3'd4;
+
+        #10
+        $finish;
+    end
+endmodule
+"""  # noqa
+
+verilog_testbench_no_reset = """\
+module tb();
+    reg clk;
+    reg[1:0] a100;
+    reg[3:0] w1;
+    reg[2:0] w12;
+    wire[1:0] out1;
+    wire[8:0] out10;
+
+    integer tb_iter;
+    toplevel block(.clk(clk), .a100(a100), .w1(w1), .w12(w12), .out1(out1), .out10(out10));
+
+    always
+        #5 clk = ~clk;
+
+    initial begin
+        $dumpfile ("waveform.vcd");
+        $dumpvars;
+
+        clk = 0;
+        block.r1 = 2;
+        block.r2 = 3;
+        block.tmp0 = 0;
+        for (tb_iter = 0; tb_iter < 32; tb_iter++) begin block.mem_0[tb_iter] = 0; end
+        block.mem_0[2] = 9;
+        block.mem_0[9] = 12;
+        a100 = 2'd0;
+        w1 = 4'd0;
+        w12 = 3'd0;
+
+        #10
+        a100 = 2'd1;
+        w1 = 4'd4;
+        w12 = 3'd1;
+
+        #10
+        a100 = 2'd3;
+        w1 = 4'd2;
+        w12 = 3'd7;
+
+        #10
+        a100 = 2'd2;
+        w1 = 4'd3;
+        w12 = 3'd4;
+
+        #10
+        $finish;
+    end
+endmodule
+"""
+
+verilog_testbench_custom_reset = """\
+module tb();
+    reg clk;
+    reg rst;
+
+    integer tb_iter;
+    toplevel block(.clk(clk), .rst(rst));
+
+    always
+        #5 clk = ~clk;
+
+    initial begin
+        $dumpfile ("waveform.vcd");
+        $dumpvars;
+
+        clk = 0;
+        block.r = 0;
+        $finish;
+    end
+endmodule
+"""
+
 
 class TestOutputTestbench(unittest.TestCase):
     def setUp(self):
         pyrtl.reset_working_block()
+        # To compare textual consistency, need to make
+        # sure we're starting at the same index for all
+        # automatically created names.
+        pyrtl.wire._reset_wire_indexers()
+        pyrtl.memory._reset_memory_indexer()
 
     def test_verilog_testbench_does_not_throw_error(self):
         zero = pyrtl.Input(1, 'zero')
@@ -1470,7 +1613,7 @@ def test_verilog_testbench_does_not_throw_error(self):
         with io.StringIO() as tbfile:
             pyrtl.output_verilog_testbench(tbfile, sim_trace)
 
-    def test_verilog_testbench_consistency(self):
+    def create_design(self):
         # Various wire names so we can verify they are printed
         # in deterministic order each time
         i1, i2, i3 = pyrtl.input_list('w1/4 w12/3 a100/2')
@@ -1499,9 +1642,38 @@ def test_verilog_testbench_consistency(self):
             'w12': [0, 1, 7, 4],
             'a100': [0, 1, 3, 2],
         })
+        return sim_trace
+
+    def test_verilog_testbench_consistency(self):
+        sim_trace = self.create_design()
         with io.StringIO() as tbfile:
             pyrtl.output_verilog_testbench(tbfile, sim_trace)
-            self.assertEqual(tbfile.getvalue(), verilog_test_bench)
+            self.assertEqual(tbfile.getvalue(), verilog_testbench)
+
+    def test_verilog_testbench_no_reset_consistency(self):
+        sim_trace = self.create_design()
+        with io.StringIO() as tbfile:
+            pyrtl.output_verilog_testbench(tbfile, sim_trace, add_reset=False)
+            self.assertEqual(tbfile.getvalue(), verilog_testbench_no_reset)
+
+    def test_error_verilog_testbench_invalid_add_reset(self):
+        tbfile = io.StringIO()
+        with self.assertRaisesRegex(pyrtl.PyrtlError, "Invalid add_reset option"):
+            pyrtl.output_verilog_testbench(tbfile, add_reset='foobar')
+
+    def test_error_verilog_testbench_existing_reset_wire(self):
+        tbfile = io.StringIO()
+        _rst = pyrtl.Input(1, 'rst')
+        with self.assertRaisesRegex(pyrtl.PyrtlError, "Found a user-defined wire named 'rst'."):
+            pyrtl.output_verilog_testbench(tbfile)
+
+    def test_verilog_testbench_existing_reset_wire_without_add_reset(self):
+        buffer = io.StringIO()
+        rst = pyrtl.Input(1, 'rst')
+        r = pyrtl.Register(4, 'r')
+        r.next <<= pyrtl.select(rst, 0, r + 1)
+        pyrtl.output_verilog_testbench(buffer, add_reset=False)
+        self.assertEqual(buffer.getvalue(), verilog_testbench_custom_reset)
 
 
 firrtl_output_concat_test = """\