leonardt / fault

A Python package for testing hardware (part of the magma ecosystem)
BSD 3-Clause "New" or "Revised" License
41 stars 13 forks source link

Add prototype sequence tester #267

Closed leonardt closed 4 years ago

leonardt commented 4 years ago

Adds a basic sequence tester pattern drawing off the UVM style pattern of sequences, drivers, and monitors.

For now, we simply enable the definitation of input and output sequences as a Python Sequence of values. Drivers and Monitors are functions that take a reference to the tester and the current input/output value. Inside the function they can lower the value into an arbitrary sequence of test actions.

There's definitely more advanced patterns we should consider, such as monitors being based purely on dynamic values (e.g. a monitor could just watch a bus and make sure transactions are valid using invariants, so it wouldn't need to be associated with an output sequence). There also may be variable length sequences (e.g. every 4 input values corresponds to 1 output value).

However, this basic pattern is intended to target the testing of the garnet PE core/tile (thus the test/example is a simple instace of this pattern). The goal is to reuse the same input/output sequence for both the core and tile. This is done by "lifting" the sequences to the tile level by defining a new driver (in this case the monitor is the same, but for a more complex example, you may need to do some different logic, e.g. read the right connection box value based on a randomized configuration).

I'm open to changes to the interface (especially with the above examples or others in mind), this was simply something basic I came up for the target use case.

leonardt commented 4 years ago

Perhaps we can call this a LockStepSequenceTester or something given the constrained target use case (every input value must match an output value)

rsetaluri commented 4 years ago

Have some thoughts on the overall design. I think the main question is how much stuff we throw into classes vs using a more pythonic free-form/untyped interface. The trade-off to me there is how much it "looks" like UVM for verif. engineers trying to use this, vs. "fixing" the problems we know exist in UVM by doing it again. A related issue is how incrementally we an build the UVM abstractions (e.g. passive vs active monitors).

I think the value of the UVM pattern itself is in the abstract-ing of monitor and driver. I think the idea of a sequence/sequencer is fairly simple. Primarily I think a useful thing would be for monitor and driver to be stateful. Therefore, I would advocate for drivers and monitors to be objects which inherit from Driver and Monitor base classes, respectively. I think for sequences we can just use duck typing on typing.Sequence, which is basically any sort of iterable object. That can definitely be implemented by a builtin python list or custom iterable class.

After looking at references on the UVM driver-monitor pattern (which admittedly are all pretty half-baked 😞), I think the attached design is minimal but extensible in that we can support:

The main deficiency I see vs. your design the explicit association of sequences with drivers/monitors. I think the main thing there is the ability to chain drivers/monitors rather than having just 1 driver and 1 monitor. But I actually think the latter is closer to the UVM abstraction. If we want the ability to chain them, I think we could have like a CompoundDriver class which just takes in multiple drivers and calls them in series.

import pytest

import magma as m
from hwtypes import BitVector

from fault.tester.staged_tester import Tester

class SequenceTester(Tester):
    def __init__(self, circuit, driver, monitor, sequence, clock=None):
        super().__init__(circuit, clock)
        self.driver = driver
        self.driver.set_tester(self)
        self.monitor = monitor
        self.monitor.set_tester(self)
        self.sequence = sequence

    def _compile_sequences(self):
        for item in self.sequence:
            self.driver.consume(*item)
            if self.clock is None:
                self.eval()
            else:
                self.step(2)
            self.monitor.observe(*item)

    def _compile(self, target="verilator", **kwargs):
        self._compile_sequences()
        super()._compile(target, **kwargs)

class ALUCore(m.Circuit):
    io = m.IO(a=m.In(m.UInt[16]),
              b=m.In(m.UInt[16]),
              opcode=m.In(m.UInt[2]),
              c=m.Out(m.UInt[16]))
    io.c @= m.mux([io.a + io.b, io.a - io.b, io.a * io.b, io.a / io.b],
                  io.opcode)

class ALUTile(m.Circuit):
    io = m.IO(a=m.In(m.UInt[16]),
              b=m.In(m.UInt[16]),
              config_data=m.In(m.UInt[2]),
              config_en=m.In(m.Enable),
              c=m.Out(m.UInt[16])) + m.ClockIO()
    config_reg = m.Register(m.Bits[2], has_enable=True)()
    config_reg.CE @= io.config_en
    config_reg.I @= io.config_data
    alu = ALUCore()
    io.c @= alu(io.a, io.b, config_reg.O)

class Driver:
    def set_tester(self, tester):
        self.tester = tester

class Monitor:
    def set_tester(self, tester):
        self.tester = tester

class CoreDriver(Driver):
    def consume(self, a, b, opcode):
        self.tester.circuit.a = a
        self.tester.circuit.b = b
        self.tester.circuit.opcode = opcode

class TileDriver(Driver):
    def consume(self, a, b, opcode):
        self.tester.circuit.config_en = 1
        self.tester.circuit.config_data = opcode
        self.tester.step(2)
        # Make sure enable logic works
        self.tester.circuit.config_en = 0
        self.tester.circuit.config_data = BitVector.random(2)
        self.tester.circuit.a = a
        self.tester.circuit.b = b

class MyMonitor(Monitor):
    def __init__(self, ops):
        self.ops = ops

    def observe(self, a, b, opcode):
        expected = self.ops[int(opcode)](a, b)
        self.tester.circuit.c.expect(expected)

# @pytest.mark.parametrize('circuit, input_driver, output_monitor, clock', [
#     (ALUCore, core_input_driver, shared_output_monitor, None),
#     (ALUTile, tile_input_driver, shared_output_monitor, ALUTile.CLK),
# ])
def test_simple_alu_sequence(): #circuit, input_driver, output_monitor, clock):
    inputs = [(BitVector.random(16), BitVector.random(16), BitVector.random(2))
              for _ in range(5)]

    ops = [
        lambda x, y: x + y,
        lambda x, y: x - y,
        lambda x, y: x * y,
        lambda x, y: x // y
    ]

    tester = SequenceTester(ALUTile,
                            TileDriver(),
                            MyMonitor(ops),
                            inputs,
                            clock=ALUTile.CLK)

    tester.compile_and_run("verilator")
leonardt commented 4 years ago

Design seems reasonable and more extensible than what I had, I can update the implementation.

I'm not so sure about the compound driver idea. Composability seems nice, but from my experience a driver is responsible for lowering a sequence item into a set of low-level test actions. We could try to sequentially compose drivers F and G on sequence item x doing F(G(x)) but that wouldn't make much sense since G would produce low level actions rather than a sequence item. The other option is do a parallel composition such as F(x) | G(x) which means both F and G lower x to low-level actions independently. This makes more sense, but it almost feels like this is driving two sequence items (identical) on separate interfaces going into a circuit. I'm struggling to see a case where such a thing might be useful, maybe some stub circuit where the logic to convert between two protocols doesn't exist, but you want the submodules to consume the same sequence item?

For parallel composition, another approach is to use standard reuse patterns like helper functions and inheritance to capture common logic across drivers, so the composition of two drivers for a test could just be defined as standard function composition. This seems like it would cover the parallel composition use case fine without adding any complexity, so I think we would need to come up with a use case where driver composition affords more capabilities or convenience over standard reuse patterns.

rsetaluri commented 4 years ago

Yeah. I'm not sure where composition would be useful or what it even means in this context (as you said there's a lot of different forms). I just wanted to address what seemed like the main difference between this design and the original implementation was. But it sounds like it wasn't a feature but just an implementation detail. So not an issue.