phanrahan / magma

magma circuits
Other
242 stars 23 forks source link

clock wiring #208

Open phanrahan opened 6 years ago

phanrahan commented 6 years ago

We should review clock wiring. Some circuit definitions call wireclocks and wiredefaultclock, but I am not sure that these are needed. There should be default clock wiring semantics that handles most cases.

leonardt commented 6 years ago

The verilog and coreir backends individually call wiredefaultclock which will do the default wiring semantics for ClockType. I think we should move these default passes into a stage of the compiler before the backends (i.e. a set of passes that are run across all backends).

Neither backend calls wireclock which handles the ResetType and EnableType, should we have those running by default?

Also, I think this is related to https://github.com/phanrahan/magma/issues/195 and https://github.com/phanrahan/magma/issues/103

One minor issue I have with the function name wireclock is that it actually doesn't wire up the ClockType (which is done by wiredefaultclock), so I think there's a better name we could use.

Alternatively, we could use wireclock as a function which takes in a set of types to wire up. So by default it wires up ClockType and the user can pass in additional types to automatically wire like EnableType and ResetType.

Also, @cdonovick mentioned that he thought EnableType and ResetType being sub types of ClockType was strange. He preferred they be sub types of Bit since it's likely you'd want to do logic on enable and reset signals.