phanrahan / mantle

mantle library
Other
42 stars 10 forks source link

[primitives] generalizing width parameter interface to a type parameter interface #123

Open leonardt opened 5 years ago

leonardt commented 5 years ago

The current interface to the mantle primitives accepts a width parameter to the generator.

This interface is limiting when encountering more complex types, such as a multi dimensional array.

For example, one may want to write the following code

# test.py
import magma as m
import mantle

class TestCircuit(m.Circuit):
    IO = ["I0", m.In(m.Array(3, m.Bits(4))),
          "I1", m.In(m.Array(3, m.Bits(4))),
          "S", m.In(m.Array(3, m.Bits(4))),
          "O", m.Out(m.Array(3, m.Bits(4)))]
    @classmethod
    def definition(io):
        io.O <= mantle.mux([io.I0, io.I1], io.S)

But when I try to run it I get


~
❯ python test.py
magma:ERROR:repos/mantle/mantle/common/operator.py:287: Cannot wire TestCircuit.Mux2xNone_inst0.I0 (type=In(Bi
t)) to I0 (type=Array(3,Out(Bits(4)))) because TestCircuit.I0 is not a _Bit
magma:ERROR:    return Mux(len(I), type_=type(I[0]))(*I, S)

magma:ERROR:repos/mantle/mantle/common/operator.py:287: Cannot wire TestCircuit.Mux2xNone_inst0.I1 (type=In(Bi
t)) to I1 (type=Array(3,Out(Bits(4)))) because TestCircuit.I1 is not a _Bit
magma:ERROR:    return Mux(len(I), type_=type(I[0]))(*I, S)

magma:ERROR:repos/mantle/mantle/common/operator.py:287: Cannot wire TestCircuit.Mux2xNone_inst0.S (type=In(Bit
)) to S (type=Array(3,Out(Bits(4)))) because TestCircuit.S is not a _Bit
magma:ERROR:    return Mux(len(I), type_=type(I[0]))(*I, S)

magma:ERROR:repos/magma/magma/t.py:72: Cannot wire TestCircuit.Mux2xNone_inst0.O (type=Out(Bit)) to TestCircui
t.O (type=Array(3,In(Bits(4)))) because TestCircuit.Mux2xNone_inst0.O is not an Array
magma:ERROR:            self.wire(other)

The underlying issue is that the mantle operator for mux expected that the input lengths are a single dimension array or single bit (so get_length returns an integer for an array or None or a Bit). However, in this case, we have a 2d array. One augmentation to support this would be to add a notion of a tuple for width, so width=(2, 4) would correspond to a 2x4 array. But this doesn't necessary extend to records.

I think the more general solution would be to change the width parameter, to a "type" parameter (either type_ or T), which specifies the magma type of the input/output. So instead of width=4, we would say type_=Bits(4). We could still provide width as a convenience parameter (which would set type_=BitsOrBit(width) under the hood), but then this would allow the user to specify something like mantle.Mux(height=2, type_=m.Tuple(m.Bit, m.Bits(3)), which I think would be a better solution than trying to build some tuple semantics into the width parameter.

leonardt commented 5 years ago

Hmm, as I dig deeper into this issue, it seems that the coreir primitives use the same width interface, so at some point the logic on these composite types need to be represented in terms of the coreir primitives (unless they are also extended to have this more "generalized" interface). It seems that we might be able to write some reuseable logic for all primitives to recursively apply these operators in terms of the leaf nodes of the type, so once it reaches coreir it's all been lowered into the wiring of Bit or Bits types.

leonardt commented 5 years ago

I've added an initial implementation for multi-dimensional arrays in the mux, see this commit https://github.com/phanrahan/mantle/commit/645946de15a5e241cc577011ac149db35ca22b39

We should be able to generalize this logic for all operators (basically, if it's not a leaf node type, then recursively generate the children and wire them up).

It should be extendable to Tuples, but I haven't implemented that yet, will probably wait until we decide on a clean design before investing more into it, unless someone needs the feature.