From 14beaa1a4862af53a3072207a7325aa10b786c39 Mon Sep 17 00:00:00 2001 From: Wanda Date: Thu, 8 Feb 2024 17:53:17 +0100 Subject: [PATCH] Implement RFC 27 amendment: deprecate `add_sync_process`, not `add_process`. --- amaranth/sim/core.py | 21 +---- docs/changes.rst | 5 +- examples/basic/ctr_en.py | 8 +- tests/test_lib_cdc.py | 24 +++--- tests/test_lib_crc.py | 22 ++--- tests/test_lib_fifo.py | 15 ++-- tests/test_sim.py | 168 ++++++++++++++++++++------------------- 7 files changed, 122 insertions(+), 141 deletions(-) diff --git a/amaranth/sim/core.py b/amaranth/sim/core.py index a50e468b9..0c7ecaa16 100644 --- a/amaranth/sim/core.py +++ b/amaranth/sim/core.py @@ -80,7 +80,6 @@ def _check_process(self, process): .format(process)) return process - @deprecated("The `add_process` method is deprecated per RFC 27. Use `add_testbench` instead.") def add_process(self, process): process = self._check_process(process) def wrapper(): @@ -89,6 +88,7 @@ def wrapper(): yield from process() self._engine.add_coroutine_process(wrapper, default_cmd=None) + @deprecated("The `add_sync_process` method is deprecated per RFC 47. Use `add_process` or `add_testbench` instead.") def add_sync_process(self, process, *, domain="sync"): process = self._check_process(process) def wrapper(): @@ -107,25 +107,6 @@ def wrapper(): except StopIteration: break try: - if isinstance(command, (Settle, Delay, Tick)): - frame = generator.gi_frame - module_globals = frame.f_globals - if '__name__' in module_globals: - module = module_globals['__name__'] - else: - module = "" - # If the warning action is "error", this call will throw the warning, and - # the try block will redirect it into the generator. - warnings.warn_explicit( - f"Using `{command.__class__.__name__}` is deprecated within " - f"`add_sync_process` per RFC 27; use `add_testbench` instead.", - DeprecationWarning, - filename=frame.f_code.co_filename, - lineno=frame.f_lineno, - module=module, - registry=module_globals.setdefault("__warningregistry__", {}), - module_globals=module_globals, - ) result = yield command exception = None except Exception as e: diff --git a/docs/changes.rst b/docs/changes.rst index 08792f84f..8d7908a92 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -29,9 +29,8 @@ Apply the following changes to code written against Amaranth 0.4 to migrate it t * Replace uses of ``Value.matches()`` with no patterns with ``Const(1)`` * Update uses of ``amaranth.utils.log2_int(need_pow2=False)`` to :func:`amaranth.utils.ceil_log2` * Update uses of ``amaranth.utils.log2_int(need_pow2=True)`` to :func:`amaranth.utils.exact_log2` -* Update uses of ``Simulator.add_process`` to ``Simulator.add_testbench`` * Convert uses of ``Simulator.add_sync_process`` used as testbenches to ``Simulator.add_testbench`` -* Convert uses of ``yield Tick()`` within remaining ``Simulator.add_sync_process`` to plain ``yield`` +* Convert other uses of ``Simulator.add_sync_process`` to ``Simulator.add_process`` Implemented RFCs @@ -81,7 +80,7 @@ Toolchain changes * Added: ``Simulator.add_testbench``. (`RFC 27`_) * Deprecated: ``Settle`` simulation command. (`RFC 27`_) -* Deprecated: ``Simulator.add_process``. (`RFC 27`_) +* Deprecated: ``Simulator.add_sync_process``. (`RFC 27`_) * Removed: (deprecated in 0.4) use of mixed-case toolchain environment variable names, such as ``NMIGEN_ENV_Diamond`` or ``AMARANTH_ENV_Diamond``; use upper-case environment variable names, such as ``AMARANTH_ENV_DIAMOND``. diff --git a/examples/basic/ctr_en.py b/examples/basic/ctr_en.py index 833b8bd21..ae67c5226 100644 --- a/examples/basic/ctr_en.py +++ b/examples/basic/ctr_en.py @@ -23,12 +23,12 @@ def elaborate(self, platform): sim = Simulator(ctr) sim.add_clock(1e-6) def ce_proc(): - yield; yield; yield + yield Tick(); yield Tick(); yield Tick() yield ctr.en.eq(1) - yield; yield; yield + yield Tick(); yield Tick(); yield Tick() yield ctr.en.eq(0) - yield; yield; yield + yield Tick(); yield Tick(); yield Tick() yield ctr.en.eq(1) -sim.add_sync_process(ce_proc) +sim.add_testbench(ce_proc) with sim.write_vcd("ctrl.vcd", "ctrl.gtkw", traces=[ctr.en, ctr.v, ctr.o]): sim.run_until(100e-6, run_passive=True) diff --git a/tests/test_lib_cdc.py b/tests/test_lib_cdc.py index a2710329e..b5439c4b3 100644 --- a/tests/test_lib_cdc.py +++ b/tests/test_lib_cdc.py @@ -26,13 +26,13 @@ def test_basic(self): def process(): self.assertEqual((yield o), 0) yield i.eq(1) - yield + yield Tick() self.assertEqual((yield o), 0) - yield + yield Tick() self.assertEqual((yield o), 0) - yield + yield Tick() self.assertEqual((yield o), 1) - sim.add_sync_process(process) + sim.add_process(process) sim.run() def test_reset_value(self): @@ -45,13 +45,13 @@ def test_reset_value(self): def process(): self.assertEqual((yield o), 1) yield i.eq(0) - yield + yield Tick() self.assertEqual((yield o), 1) - yield + yield Tick() self.assertEqual((yield o), 1) - yield + yield Tick() self.assertEqual((yield o), 0) - sim.add_sync_process(process) + sim.add_process(process) sim.run() @@ -221,17 +221,17 @@ def process(): yield ps.i.eq(0) # TODO: think about reset for n in range(5): - yield + yield Tick() # Make sure no pulses are generated in quiescent state for n in range(3): - yield + yield Tick() self.assertEqual((yield ps.o), 0) # Check conservation of pulses accum = 0 for n in range(10): yield ps.i.eq(1 if n < 4 else 0) - yield + yield Tick() accum += yield ps.o self.assertEqual(accum, 4) - sim.add_sync_process(process) + sim.add_process(process) sim.run() diff --git a/tests/test_lib_crc.py b/tests/test_lib_crc.py index db06442af..96b57fb17 100644 --- a/tests/test_lib_crc.py +++ b/tests/test_lib_crc.py @@ -243,13 +243,13 @@ def process(): yield crc.start.eq(word == b"1") yield crc.data.eq(word) yield crc.valid.eq(1) - yield + yield Tick() yield crc.valid.eq(0) - yield + yield Tick() self.assertEqual((yield crc.crc), check) sim = Simulator(crc) - sim.add_sync_process(process) + sim.add_testbench(process) sim.add_clock(1e-6) sim.run() @@ -283,18 +283,18 @@ def test_crc_words(self): def process(): yield crc.start.eq(1) - yield + yield Tick() yield crc.start.eq(0) for word in words: yield crc.data.eq(word) yield crc.valid.eq(1) - yield + yield Tick() yield crc.valid.eq(0) - yield + yield Tick() self.assertEqual((yield crc.crc), check) sim = Simulator(crc) - sim.add_sync_process(process) + sim.add_testbench(process) sim.add_clock(1e-6) sim.run() @@ -334,17 +334,17 @@ def test_crc_match(self): def process(): yield crc.start.eq(1) - yield + yield Tick() yield crc.start.eq(0) for word in words: yield crc.data.eq(word) yield crc.valid.eq(1) - yield + yield Tick() yield crc.valid.eq(0) - yield + yield Tick() self.assertTrue((yield crc.match_detected)) sim = Simulator(crc) - sim.add_sync_process(process) + sim.add_testbench(process) sim.add_clock(1e-6) sim.run() diff --git a/tests/test_lib_fifo.py b/tests/test_lib_fifo.py index e35c978f8..6ce87056b 100644 --- a/tests/test_lib_fifo.py +++ b/tests/test_lib_fifo.py @@ -296,7 +296,7 @@ def testbench(): for i in range(10): yield fifo.w_data.eq(i) yield fifo.w_en.eq(1) - yield + yield Tick() if (i - ff_syncronizer_latency) > 0: self.assertEqual((yield fifo.r_level), i - ff_syncronizer_latency) @@ -305,7 +305,7 @@ def testbench(): simulator = Simulator(fifo) simulator.add_clock(100e-6) - simulator.add_sync_process(testbench) + simulator.add_process(testbench) simulator.run() def check_async_fifo_level(self, fifo, fill_in, expected_level, read=False): @@ -315,10 +315,9 @@ def write_process(): for i in range(fill_in): yield fifo.w_data.eq(i) yield fifo.w_en.eq(1) - yield + yield Tick("write") yield fifo.w_en.eq(0) - yield - yield + yield Tick ("write") self.assertEqual((yield fifo.w_level), expected_level) yield write_done.eq(1) @@ -326,14 +325,14 @@ def read_process(): if read: yield fifo.r_en.eq(1) while not (yield write_done): - yield + yield Tick("read") self.assertEqual((yield fifo.r_level), expected_level) simulator = Simulator(fifo) simulator.add_clock(100e-6, domain="write") - simulator.add_sync_process(write_process, domain="write") + simulator.add_testbench(write_process) simulator.add_clock(50e-6, domain="read") - simulator.add_sync_process(read_process, domain="read") + simulator.add_testbench(read_process) with simulator.write_vcd("test.vcd"): simulator.run() diff --git a/tests/test_sim.py b/tests/test_sim.py index 61e3dbb7d..fd90e8d60 100644 --- a/tests/test_sim.py +++ b/tests/test_sim.py @@ -435,29 +435,30 @@ def setUp_counter(self): def test_counter_process(self): self.setUp_counter() - with _ignore_deprecated(): - with self.assertSimulation(self.m) as sim: - def process(): - self.assertEqual((yield self.count), 4) + with self.assertSimulation(self.m) as sim: + def process(): + self.assertEqual((yield self.count), 4) + yield Delay(1e-6) + self.assertEqual((yield self.count), 4) + yield self.sync.clk.eq(1) + self.assertEqual((yield self.count), 4) + with _ignore_deprecated(): + yield Settle() + self.assertEqual((yield self.count), 5) + yield Delay(1e-6) + self.assertEqual((yield self.count), 5) + yield self.sync.clk.eq(0) + self.assertEqual((yield self.count), 5) + with _ignore_deprecated(): + yield Settle() + self.assertEqual((yield self.count), 5) + for _ in range(3): yield Delay(1e-6) - self.assertEqual((yield self.count), 4) yield self.sync.clk.eq(1) - self.assertEqual((yield self.count), 4) - yield Settle() - self.assertEqual((yield self.count), 5) yield Delay(1e-6) - self.assertEqual((yield self.count), 5) yield self.sync.clk.eq(0) - self.assertEqual((yield self.count), 5) - yield Settle() - self.assertEqual((yield self.count), 5) - for _ in range(3): - yield Delay(1e-6) - yield self.sync.clk.eq(1) - yield Delay(1e-6) - yield self.sync.clk.eq(0) - self.assertEqual((yield self.count), 0) - sim.add_process(process) + self.assertEqual((yield self.count), 0) + sim.add_process(process) def test_counter_clock_and_sync_process(self): self.setUp_counter() @@ -472,7 +473,8 @@ def process(): for _ in range(3): yield self.assertEqual((yield self.count), 0) - sim.add_sync_process(process) + with _ignore_deprecated(): + sim.add_sync_process(process) def test_reset(self): self.setUp_counter() @@ -481,14 +483,15 @@ def test_reset(self): times = 0 def process(): nonlocal times + yield Tick() self.assertEqual((yield self.count), 4) - yield + yield Tick() self.assertEqual((yield self.count), 5) - yield + yield Tick() self.assertEqual((yield self.count), 6) - yield + yield Tick() times += 1 - sim.add_sync_process(process) + sim.add_process(process) sim.run() sim.reset() sim.run() @@ -520,19 +523,37 @@ def test_alu(self): def process(): yield self.a.eq(5) yield self.b.eq(1) - yield + yield Tick() self.assertEqual((yield self.x), 4) - yield + yield Tick() self.assertEqual((yield self.o), 6) yield self.s.eq(1) - yield - yield + yield Tick() + yield Tick() self.assertEqual((yield self.o), 4) yield self.s.eq(2) - yield - yield + yield Tick() + yield Tick() self.assertEqual((yield self.o), 0) - sim.add_sync_process(process) + sim.add_process(process) + + def test_alu_bench(self): + self.setUp_alu() + with self.assertSimulation(self.m) as sim: + sim.add_clock(1e-6) + def process(): + yield self.a.eq(5) + yield self.b.eq(1) + self.assertEqual((yield self.x), 4) + yield Tick() + self.assertEqual((yield self.o), 6) + yield self.s.eq(1) + yield Tick() + self.assertEqual((yield self.o), 4) + yield self.s.eq(2) + yield Tick() + self.assertEqual((yield self.o), 0) + sim.add_testbench(process) def setUp_clock_phase(self): self.m = Module() @@ -543,10 +564,10 @@ def setUp_clock_phase(self): self.check = self.m.domains.check = ClockDomain() self.expected = [ - [0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0], - [0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1], - [0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1], - [0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0], + [0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0], + [0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1], + [0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 1, 1], + [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0], ] def test_clock_phase(self): @@ -567,11 +588,11 @@ def proc(): self.phase270.clk ] for i in range(16): - yield + yield Tick("check") for j, c in enumerate(clocks): self.assertEqual((yield c), self.expected[j][i]) - sim.add_sync_process(proc, domain="check") + sim.add_process(proc) def setUp_multiclock(self): self.sys = ClockDomain() @@ -588,15 +609,15 @@ def test_multiclock(self): def sys_process(): yield Passive() - yield - yield + yield Tick("sys") + yield Tick("sys") self.fail() def pix_process(): - yield - yield - yield - sim.add_sync_process(sys_process, domain="sys") - sim.add_sync_process(pix_process, domain="pix") + yield Tick("pix") + yield Tick("pix") + yield Tick("pix") + sim.add_testbench(sys_process) + sim.add_testbench(pix_process) def setUp_lhs_rhs(self): self.i = Signal(8) @@ -644,7 +665,7 @@ def test_add_process_wrong(self): with self.assertSimulation(Module()) as sim: with self.assertRaisesRegex(TypeError, r"^Cannot add a process 1 because it is not a generator function$"): - sim.add_sync_process(1) + sim.add_process(1) def test_add_process_wrong_generator(self): with self.assertSimulation(Module()) as sim: @@ -652,7 +673,7 @@ def test_add_process_wrong_generator(self): r"^Cannot add a process <.+?> because it is not a generator function$"): def process(): yield Delay() - sim.add_sync_process(process()) + sim.add_process(process()) def test_add_clock_wrong_twice(self): m = Module() @@ -678,31 +699,14 @@ def test_add_clock_if_exists(self): def test_command_wrong(self): survived = False - with _ignore_deprecated(): - with self.assertSimulation(Module()) as sim: - def process(): - nonlocal survived - with self.assertRaisesRegex(TypeError, - r"Received unsupported command 1 from process .+?"): - yield 1 - survived = True - sim.add_process(process) - self.assertTrue(survived) - - def test_sync_command_deprecated(self): - survived = False - m = Module() - dummy = Signal() - m.d.sync += dummy.eq(1) - with self.assertSimulation(m) as sim: + with self.assertSimulation(Module()) as sim: def process(): nonlocal survived - with self.assertWarnsRegex(DeprecationWarning, - r"Using `Delay` is deprecated within `add_sync_process`"): - yield Delay(1e-8) + with self.assertRaisesRegex(TypeError, + r"Received unsupported command 1 from process .+?"): + yield 1 survived = True - sim.add_sync_process(process) - sim.add_clock(1e-6) + sim.add_process(process) self.assertTrue(survived) def test_sync_command_wrong(self): @@ -717,7 +721,8 @@ def process(): r"Received unsupported command 1 from process .+?"): yield 1 survived = True - sim.add_sync_process(process) + with _ignore_deprecated(): + sim.add_sync_process(process) sim.add_clock(1e-6) self.assertTrue(survived) @@ -762,15 +767,13 @@ def test_memory_init(self): with self.assertSimulation(self.m) as sim: def process(): yield self.rdport.addr.eq(1) - yield - yield + yield Tick() self.assertEqual((yield self.rdport.data), 0x55) yield self.rdport.addr.eq(2) - yield - yield + yield Tick() self.assertEqual((yield self.rdport.data), 0x00) sim.add_clock(1e-6) - sim.add_sync_process(process) + sim.add_testbench(process) def test_memory_write(self): self.setUp_memory() @@ -779,13 +782,13 @@ def process(): yield self.wrport.addr.eq(4) yield self.wrport.data.eq(0x33) yield self.wrport.en.eq(1) - yield + yield Tick() yield self.wrport.en.eq(0) yield self.rdport.addr.eq(4) - yield + yield Tick() self.assertEqual((yield self.rdport.data), 0x33) sim.add_clock(1e-6) - sim.add_sync_process(process) + sim.add_testbench(process) def test_memory_write_granularity(self): self.setUp_memory(wr_granularity=4) @@ -864,14 +867,13 @@ def test_memory_read_only(self): self.m.submodules.rdport = self.rdport = self.memory.read_port() with self.assertSimulation(self.m) as sim: def process(): - yield + yield Tick() self.assertEqual((yield self.rdport.data), 0xaa) yield self.rdport.addr.eq(1) - yield - yield + yield Tick() self.assertEqual((yield self.rdport.data), 0x55) sim.add_clock(1e-6) - sim.add_sync_process(process) + sim.add_testbench(process) def test_comb_bench_process(self): m = Module() @@ -1022,11 +1024,11 @@ def process(): self.assertEqual((yield self.memory[Const(2)]), 0x00) yield self.memory[Const(1)].eq(Const(0x33)) self.assertEqual((yield self.memory[Const(1)]), 0x55) - yield + yield Tick() self.assertEqual((yield self.memory[Const(1)]), 0x33) sim.add_clock(1e-6) - sim.add_sync_process(process) + sim.add_process(process) def test_vcd_wrong_nonzero_time(self): s = Signal()