From 5cf1029a4a4a323a23edaf5911334d8d0a370f25 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sun, 21 Apr 2019 08:52:57 +0000 Subject: [PATCH] hdl.ir: detect elaboratables that are created but not used. Requres every elaboratable to inherit from Elaboratable, but still accepts ones that do not, with a warning. Fixes #3. --- examples/alu.py | 2 +- examples/alu_hier.py | 6 ++--- examples/arst.py | 2 +- examples/ctr.py | 2 +- examples/ctr_ce.py | 2 +- examples/fsm.py | 2 +- examples/gpio.py | 2 +- examples/inst.py | 2 +- examples/mem.py | 2 +- examples/pmux.py | 2 +- nmigen/__init__.py | 2 +- nmigen/hdl/dsl.py | 4 +-- nmigen/hdl/ir.py | 45 +++++++++++++++++++++++++++------- nmigen/hdl/mem.py | 6 ++--- nmigen/hdl/xfrm.py | 2 +- nmigen/lib/cdc.py | 4 +-- nmigen/lib/coding.py | 10 ++++---- nmigen/lib/fifo.py | 8 +++--- nmigen/test/__init__.py | 6 +++++ nmigen/test/test_hdl_xfrm.py | 2 +- nmigen/test/test_lib_coding.py | 5 ++-- nmigen/test/test_lib_fifo.py | 6 ++--- 22 files changed, 79 insertions(+), 45 deletions(-) diff --git a/examples/alu.py b/examples/alu.py index 211acd2..5c0e8e6 100644 --- a/examples/alu.py +++ b/examples/alu.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.cli import main -class ALU: +class ALU(Elaboratable): def __init__(self, width): self.sel = Signal(2) self.a = Signal(width) diff --git a/examples/alu_hier.py b/examples/alu_hier.py index fc6beae..a3273af 100644 --- a/examples/alu_hier.py +++ b/examples/alu_hier.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.cli import main -class Adder: +class Adder(Elaboratable): def __init__(self, width): self.a = Signal(width) self.b = Signal(width) @@ -14,7 +14,7 @@ class Adder: return m -class Subtractor: +class Subtractor(Elaboratable): def __init__(self, width): self.a = Signal(width) self.b = Signal(width) @@ -26,7 +26,7 @@ class Subtractor: return m -class ALU: +class ALU(Elaboratable): def __init__(self, width): self.op = Signal() self.a = Signal(width) diff --git a/examples/arst.py b/examples/arst.py index 405857b..ef3ed5a 100644 --- a/examples/arst.py +++ b/examples/arst.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.cli import main -class ClockDivisor: +class ClockDivisor(Elaboratable): def __init__(self, factor): self.v = Signal(factor) self.o = Signal() diff --git a/examples/ctr.py b/examples/ctr.py index 9505a61..9752299 100644 --- a/examples/ctr.py +++ b/examples/ctr.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.cli import main, pysim -class Counter: +class Counter(Elaboratable): def __init__(self, width): self.v = Signal(width, reset=2**width-1) self.o = Signal() diff --git a/examples/ctr_ce.py b/examples/ctr_ce.py index ebf1db0..f839d67 100644 --- a/examples/ctr_ce.py +++ b/examples/ctr_ce.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.back import rtlil, verilog, pysim -class Counter: +class Counter(Elaboratable): def __init__(self, width): self.v = Signal(width, reset=2**width-1) self.o = Signal() diff --git a/examples/fsm.py b/examples/fsm.py index 98f5045..73e6f7d 100644 --- a/examples/fsm.py +++ b/examples/fsm.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.cli import main -class UARTReceiver: +class UARTReceiver(Elaboratable): def __init__(self, divisor): self.divisor = divisor diff --git a/examples/gpio.py b/examples/gpio.py index c234ce0..3dd0da0 100644 --- a/examples/gpio.py +++ b/examples/gpio.py @@ -3,7 +3,7 @@ from nmigen import * from nmigen.cli import main -class GPIO: +class GPIO(Elaboratable): def __init__(self, pins, bus): self.pins = pins self.bus = bus diff --git a/examples/inst.py b/examples/inst.py index 227a5da..2fc519b 100644 --- a/examples/inst.py +++ b/examples/inst.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.cli import main -class System: +class System(Elaboratable): def __init__(self): self.adr = Signal(16) self.dat_r = Signal(8) diff --git a/examples/mem.py b/examples/mem.py index 1d97042..82105fc 100644 --- a/examples/mem.py +++ b/examples/mem.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.cli import main -class RegisterFile: +class RegisterFile(Elaboratable): def __init__(self): self.adr = Signal(4) self.dat_r = Signal(8) diff --git a/examples/pmux.py b/examples/pmux.py index 02a6155..1e938b5 100644 --- a/examples/pmux.py +++ b/examples/pmux.py @@ -2,7 +2,7 @@ from nmigen import * from nmigen.cli import main -class ParMux: +class ParMux(Elaboratable): def __init__(self, width): self.s = Signal(3) self.a = Signal(width) diff --git a/nmigen/__init__.py b/nmigen/__init__.py index bc1560d..b5899d7 100644 --- a/nmigen/__init__.py +++ b/nmigen/__init__.py @@ -1,7 +1,7 @@ from .hdl.ast import Value, Const, C, Mux, Cat, Repl, Array, Signal, ClockSignal, ResetSignal from .hdl.dsl import Module from .hdl.cd import ClockDomain -from .hdl.ir import Fragment, Instance +from .hdl.ir import Elaboratable, Fragment, Instance from .hdl.mem import Memory from .hdl.rec import Record from .hdl.xfrm import ResetInserter, CEInserter diff --git a/nmigen/hdl/dsl.py b/nmigen/hdl/dsl.py index 95f6eb0..03e7e23 100644 --- a/nmigen/hdl/dsl.py +++ b/nmigen/hdl/dsl.py @@ -9,7 +9,7 @@ from .ir import * from .xfrm import * -__all__ = ["Module", "SyntaxError", "SyntaxWarning"] +__all__ = ["SyntaxError", "SyntaxWarning", "Module"] class SyntaxError(Exception): @@ -109,7 +109,7 @@ class FSM: return self.state == self.encoding[name] -class Module(_ModuleBuilderRoot): +class Module(_ModuleBuilderRoot, Elaboratable): def __init__(self): _ModuleBuilderRoot.__init__(self, self, depth=0) self.submodules = _ModuleBuilderSubmodules(self) diff --git a/nmigen/hdl/ir.py b/nmigen/hdl/ir.py index 079bba2..164f558 100644 --- a/nmigen/hdl/ir.py +++ b/nmigen/hdl/ir.py @@ -1,12 +1,30 @@ -import warnings +from abc import ABCMeta, abstractmethod from collections import defaultdict, OrderedDict +import warnings +import traceback +import sys from ..tools import * from .ast import * from .cd import * -__all__ = ["Fragment", "Instance", "DriverConflict"] +__all__ = ["Elaboratable", "DriverConflict", "Fragment", "Instance"] + + +class Elaboratable(metaclass=ABCMeta): + def __new__(cls, *args, **kwargs): + self = super().__new__(cls) + self._Elaboratable__traceback = traceback.extract_stack()[:-1] + self._Elaboratable__used = False + return self + + def __del__(self): + if hasattr(self, "_Elaboratable__used") and not self._Elaboratable__used: + print("Elaboratable created but never used\n", + "Traceback (most recent call last):\n", + *traceback.format_list(self._Elaboratable__traceback), + file=sys.stderr, sep="") class DriverConflict(UserWarning): @@ -16,13 +34,22 @@ class DriverConflict(UserWarning): class Fragment: @staticmethod def get(obj, platform): - if isinstance(obj, Fragment): - return obj - elif hasattr(obj, "elaborate"): - frag = obj.elaborate(platform) - else: - raise AttributeError("Object '{!r}' cannot be elaborated".format(obj)) - return Fragment.get(frag, platform) + while True: + if isinstance(obj, Fragment): + return obj + elif isinstance(obj, Elaboratable): + obj._Elaboratable__used = True + obj = obj.elaborate(platform) + elif hasattr(obj, "elaborate"): + warnings.warn( + message="Class {!r} is an elaboratable that does not explicitly inherit from " + "Elaboratable; doing so would improve diagnostics" + .format(type(obj)), + category=RuntimeWarning, + stacklevel=2) + obj = obj.elaborate(platform) + else: + raise AttributeError("Object '{!r}' cannot be elaborated".format(obj)) def __init__(self): self.ports = SignalDict() diff --git a/nmigen/hdl/mem.py b/nmigen/hdl/mem.py index bef8035..076d284 100644 --- a/nmigen/hdl/mem.py +++ b/nmigen/hdl/mem.py @@ -1,6 +1,6 @@ from .. import tracer from .ast import * -from .ir import Instance +from .ir import Elaboratable, Instance __all__ = ["Memory", "ReadPort", "WritePort", "DummyPort"] @@ -70,7 +70,7 @@ class Memory: return self._array[index] -class ReadPort: +class ReadPort(Elaboratable): def __init__(self, memory, domain, synchronous, transparent): self.memory = memory self.domain = domain @@ -135,7 +135,7 @@ class ReadPort: return f -class WritePort: +class WritePort(Elaboratable): def __init__(self, memory, domain, priority, granularity): self.memory = memory self.domain = domain diff --git a/nmigen/hdl/xfrm.py b/nmigen/hdl/xfrm.py index 1385ce8..c554bc6 100644 --- a/nmigen/hdl/xfrm.py +++ b/nmigen/hdl/xfrm.py @@ -292,7 +292,7 @@ class FragmentTransformer: raise AttributeError("Object '{!r}' cannot be elaborated".format(value)) -class TransformedElaboratable: +class TransformedElaboratable(Elaboratable): def __init__(self, elaboratable): assert hasattr(elaboratable, "elaborate") diff --git a/nmigen/lib/cdc.py b/nmigen/lib/cdc.py index 3018b37..ec83317 100644 --- a/nmigen/lib/cdc.py +++ b/nmigen/lib/cdc.py @@ -4,7 +4,7 @@ from .. import * __all__ = ["MultiReg", "ResetSynchronizer"] -class MultiReg: +class MultiReg(Elaboratable): """Resynchronise a signal to a different clock domain. Consists of a chain of flip-flops. Eliminates metastabilities at the output, but provides @@ -69,7 +69,7 @@ class MultiReg: return m -class ResetSynchronizer: +class ResetSynchronizer(Elaboratable): def __init__(self, arst, domain="sync", n=2): self.arst = arst self.domain = domain diff --git a/nmigen/lib/coding.py b/nmigen/lib/coding.py index e06d5a7..eed02c9 100644 --- a/nmigen/lib/coding.py +++ b/nmigen/lib/coding.py @@ -10,7 +10,7 @@ __all__ = [ ] -class Encoder: +class Encoder(Elaboratable): """Encode one-hot to binary. If one bit in ``i`` is asserted, ``n`` is low and ``o`` indicates the asserted bit. @@ -48,7 +48,7 @@ class Encoder: return m -class PriorityEncoder: +class PriorityEncoder(Elaboratable): """Priority encode requests to binary. If any bit in ``i`` is asserted, ``n`` is low and ``o`` indicates the least significant @@ -85,7 +85,7 @@ class PriorityEncoder: return m -class Decoder: +class Decoder(Elaboratable): """Decode binary to one-hot. If ``n`` is low, only the ``i``th bit in ``o`` is asserted. @@ -130,7 +130,7 @@ class PriorityDecoder(Decoder): """ -class GrayEncoder: +class GrayEncoder(Elaboratable): """Encode binary to Gray code. Parameters @@ -157,7 +157,7 @@ class GrayEncoder: return m -class GrayDecoder: +class GrayDecoder(Elaboratable): """Decode Gray code to binary. Parameters diff --git a/nmigen/lib/fifo.py b/nmigen/lib/fifo.py index e484ec8..2844a05 100644 --- a/nmigen/lib/fifo.py +++ b/nmigen/lib/fifo.py @@ -102,7 +102,7 @@ def _decr(signal, modulo): return Mux(signal == 0, modulo - 1, signal - 1) -class SyncFIFO(FIFOInterface): +class SyncFIFO(Elaboratable, FIFOInterface): __doc__ = FIFOInterface._doc_template.format( description=""" Synchronous first in, first out queue. @@ -209,7 +209,7 @@ class SyncFIFO(FIFOInterface): return m -class SyncFIFOBuffered(FIFOInterface): +class SyncFIFOBuffered(Elaboratable, FIFOInterface): __doc__ = FIFOInterface._doc_template.format( description=""" Buffered synchronous first in, first out queue. @@ -265,7 +265,7 @@ class SyncFIFOBuffered(FIFOInterface): return m -class AsyncFIFO(FIFOInterface): +class AsyncFIFO(Elaboratable, FIFOInterface): __doc__ = FIFOInterface._doc_template.format( description=""" Asynchronous first in, first out queue. @@ -361,7 +361,7 @@ class AsyncFIFO(FIFOInterface): return m -class AsyncFIFOBuffered(FIFOInterface): +class AsyncFIFOBuffered(Elaboratable, FIFOInterface): __doc__ = FIFOInterface._doc_template.format( description=""" Buffered asynchronous first in, first out queue. diff --git a/nmigen/test/__init__.py b/nmigen/test/__init__.py index e69de29..724ad8a 100644 --- a/nmigen/test/__init__.py +++ b/nmigen/test/__init__.py @@ -0,0 +1,6 @@ +from ..hdl.ir import Elaboratable + + +# The nMigen testsuite creates a lot of elaboratables that are intentionally unused. +# Disable the unused elaboratable check, as in our case it provides nothing but noise. +del Elaboratable.__del__ diff --git a/nmigen/test/test_hdl_xfrm.py b/nmigen/test/test_hdl_xfrm.py index 94af1f8..3973b8d 100644 --- a/nmigen/test/test_hdl_xfrm.py +++ b/nmigen/test/test_hdl_xfrm.py @@ -488,7 +488,7 @@ class CEInserterTestCase(FHDLTestCase): """) -class _MockElaboratable: +class _MockElaboratable(Elaboratable): def __init__(self): self.s1 = Signal() diff --git a/nmigen/test/test_lib_coding.py b/nmigen/test/test_lib_coding.py index 83b7831..8c6b624 100644 --- a/nmigen/test/test_lib_coding.py +++ b/nmigen/test/test_lib_coding.py @@ -1,6 +1,7 @@ from .tools import * from ..hdl.ast import * from ..hdl.dsl import * +from ..hdl.ir import * from ..back.pysim import * from ..lib.coding import * @@ -82,7 +83,7 @@ class DecoderTestCase(FHDLTestCase): sim.run() -class ReversibleSpec: +class ReversibleSpec(Elaboratable): def __init__(self, encoder_cls, decoder_cls, args): self.encoder_cls = encoder_cls self.decoder_cls = decoder_cls @@ -99,7 +100,7 @@ class ReversibleSpec: return m -class HammingDistanceSpec: +class HammingDistanceSpec(Elaboratable): def __init__(self, distance, encoder_cls, args): self.distance = distance self.encoder_cls = encoder_cls diff --git a/nmigen/test/test_lib_fifo.py b/nmigen/test/test_lib_fifo.py index c7f4c28..3497186 100644 --- a/nmigen/test/test_lib_fifo.py +++ b/nmigen/test/test_lib_fifo.py @@ -45,7 +45,7 @@ class FIFOSmokeTestCase(FHDLTestCase): self.assertAsyncFIFOWorks(AsyncFIFOBuffered(width=8, depth=3)) -class FIFOModel(FIFOInterface): +class FIFOModel(Elaboratable, FIFOInterface): """ Non-synthesizable first-in first-out queue, implemented naively as a chain of registers. """ @@ -104,7 +104,7 @@ class FIFOModel(FIFOInterface): return m -class FIFOModelEquivalenceSpec: +class FIFOModelEquivalenceSpec(Elaboratable): """ The first-in first-out queue model equivalence specification: for any inputs and control signals, the behavior of the implementation under test exactly matches the ideal model, @@ -148,7 +148,7 @@ class FIFOModelEquivalenceSpec: return m -class FIFOContractSpec: +class FIFOContractSpec(Elaboratable): """ The first-in first-out queue contract specification: if two elements are written to the queue consecutively, they must be read out consecutively at some later point, no matter all other -- 2.30.2