phanrahan / magma

magma circuits
Other
253 stars 24 forks source link

[IO] Unintuitive behavior with IO objects #1349

Open leonardt opened 10 months ago

leonardt commented 10 months ago

Here's an example of some code that doesn't work because of the immutable nature of IO objects.

class RegisterFile(m.Generator):
    def __init__(self, n: int, T: m.Type, has_read_enable: bool = False):
        addr_len = m.bitutils.clog2(n)
        self.io = m.IO(
            raddr=m.In(m.Bits[addr_len]),
            rdata=m.Out(T),
            wen=m.In(m.Enable),
            waddr=m.In(m.Bits[addr_len]),
            wdata=m.In(T)
        )
        rdata = self.io.rdata
        if has_read_enable:
            self.io += m.IO(ren=m.In(m.Enable))
            rdata_reg = m.Register(T)()
            rdata @= rdata_reg.O

The problem here is that the rdata reference is invalidated when the new IO object is created to add the port.

We should at least raise a useful error here (e.g. "Attempting to wire to an invalidated IO object"), although I think we might consider whether we should make IO object mutable to make this seemingly natural code work.

Is there a strong argument for immutability of IO objects? i.e. is there a class of errors we'd like to avoid that immutability guarantees us against? As it currently stands, immutability actually introduces a new class of possible errors, so we'd want a strong argument for why these errors are useful for users (i.e. how they can avoid users introducing more difficult to fix errors).

rsetaluri commented 10 months ago

I agree; I think immutability only needs to kick in once the circuit definition is finished, i.e. open() should not permit modifying the IO. I can make these changes to allow the IO to be mutable.