Issue with compiling AsyncReset to CoreIR

[rsetaluri]

The following snippet

>>> import magma
>>>
>>> foo = magma.DefineCircuit("foo", "r", magma.In(magma.AsyncReset))
>>> magma.EndCircuit()
>>> 
>>> top = magma.DefineCircuit("top", "O", magma.Out(magma.Bit))
>>> foo_inst = foo()
>>> magma.wire(magma.asyncreset(0), foo_inst.r)
>>> magma.wire(magma.bit(0), top.O)
>>> magma.EndCircuit()
>>> magma.compile("top", top, output="coreir")

results in the following error:

ERROR: top: Cannot wire together
  bit_const_0_None.out : Bit
  inst0.r : coreir.arstIn


I AM DYING!
Assertion failed: (0), function die, file context.cpp, line 112.
Abort trap: 6

[rsetaluri]

It turns out this issue exists for all Bit subtypes: Clock, Reset, and AsyncReset

[leonardt]

Yes, the issue here is that the magma.asynceset conversion doesn't preserve anything in the graph we compile. For this to work in coreir, we need to use a wrap module (see https://github.com/phanrahan/mantle/blob/master/mantle/coreir/FF.py#L95-L107) which converts the in_type (in this case Bit) to out_type (in this case AsyncReset).

I'll look into this, perhaps we can assert that if it passes the magma wiring logic (where type checking occurs), then we can assume that we need to do insert the wrap node when we encounter the different types. Alternatively, when traversing the connection, we may be able to find information that tells us the conversion occured.

[leonardt]

One workaround until this is fixed is to use the wrap module explicitly when converting between Bit and any of the named types.

[rsetaluri]

Got it. I went down the path of wrapping explicitly when trying to convert. I used the code in FF.py and this works for AsyncReset but did not work for any of the other clock types (e.g. Clock). I changed line 103 to the proper type.

To be clear; this works:

from magma import *

def define_wrap(type_, type_name, in_type):
    def sim_wrap(self, value_store, state_store):
        input_val = value_store.get_value(getattr(self, "in"))
        value_store.set_value(self.out, input_val)

    return DeclareCircuit(
        f'coreir_wrap{type_name}',
        "in", In(in_type), "out", Out(type_),
        coreir_genargs = {"type": AsyncReset},
        coreir_name="wrap",
        coreir_lib="coreir",
        simulate=sim_wrap
    )

foo = DefineCircuit("foo", "r", In(AsyncReset))
EndCircuit()

top = DefineCircuit("top", "O", Out(Bit))
foo_inst = foo()

wrap = define_wrap(AsyncReset, "Bit", Bit)()
wire(bit(0), wrap.interface.ports["in"])
wire(wrap.out, foo_inst.r)

wire(bit(0), top.O)
EndCircuit()
compile("top", top, output="coreir")

but this does not

from magma import *

def define_wrap(type_, type_name, in_type):
    def sim_wrap(self, value_store, state_store):
        input_val = value_store.get_value(getattr(self, "in"))
        value_store.set_value(self.out, input_val)

    return DeclareCircuit(
        f'coreir_wrap{type_name}',
        "in", In(in_type), "out", Out(type_),
        coreir_genargs = {"type": Clock},
        coreir_name="wrap",
        coreir_lib="coreir",
        simulate=sim_wrap
    )

foo = DefineCircuit("foo", "r", In(Clock))
EndCircuit()

top = DefineCircuit("top", "O", Out(Bit))
foo_inst = foo()

wrap = define_wrap(Clock, "Bit", Bit)()
wire(bit(0), wrap.interface.ports["in"])
wire(wrap.out, foo_inst.r)

wire(bit(0), top.O)
EndCircuit()
compile("top", top, output="coreir")

And the error is

Traceback (most recent call last):
  File "test.py", line 29, in <module>
    compile("top", top, output="coreir")
  File "/Users/setaluri/dev/magma/magma/compile.py", line 112, in compile
    __compile_to_coreir(main, file_name, opts)
  File "/Users/setaluri/dev/magma/magma/compile.py", line 66, in __compile_to_coreir
    backend.compile(main)
  File "/Users/setaluri/dev/magma/magma/backend/coreir_.py", line 395, in compile
    self.modules[defn.name] = self.compile_definition(defn)
  File "/Users/setaluri/dev/magma/magma/backend/coreir_.py", line 288, in compile_definition
    self.compile_definition_to_module_definition(definition, module_definition)
  File "/Users/setaluri/dev/magma/magma/backend/coreir_.py", line 254, in compile_definition_to_module_definition
    coreir_instance = self.compile_instance(instance, module_definition)
  File "/Users/setaluri/dev/magma/magma/backend/coreir_.py", line 214, in compile_instance
    gen_args = self.context.new_values(gen_args)
  File "/Users/setaluri/dev/pycoreir/coreir/context.py", line 126, in new_values
    raise NotImplementedError(v, type(v))
NotImplementedError: (Clock, <class 'magma.clock.ClockKind'>)

[rsetaluri]

Ok, looks like with #289 it works now for clocks. I believe for Reset and Enable type we don't need to do the wrapping at all.

[leonardt]

Closing because of #289 feel free to reopen if there's still an issue.

[leonardt]

Reset and Enable don't have named types in coreir, so they just map to Bit. I forgot to include that the reason we need to use wrap is for converting from Bit to named types (since they aren't subtypes of a parent _Bit type like we have in magma, we have to explicitly convert between them).

[rsetaluri]

Right. I also added a test to make sure that wire(reset(0), foo.rst) works and compiles (same with enable).