magma simulator about magma HOT 19 CLOSED

@shacklettbp Was using this issue to track the integration of your simulator. Feel free to refine the top-level comment with any changes if you think the any parts integration should be done differently. (Let me know if you can't edit it)

from magma.

@leonardt, Here is my rough idea for how the simulator could fit into the overall architecture of the project. I'll edit the top level comment when we have something of a consensus on how the design might look. Let me know your thoughts:

• An abstract class CircuitSimulator defines the interface for the simulator. The python simulator would implement this interface (with a PythonSimulator class), but a CoreIR simulator or even a debugging tool running on the live FPGA could be added as backends. This class's constructor would accept the main module of the magma circuit as an argument so the backend can process the circuit and store it's internal representation.
• The debugger frontend (and potentially another frontend designed for automated testing) would use the CircuitSimulator's interface to interact with the the underlying simulator. The interface could include methods to query the capabilities of the underlying simulator, since the Python and CoreIR simulators would likely have different limitations.
• CircuitSimulator would define get_value(bit), which returns the current simulated value of a given Bit object. This function could return None in the case where the Bit doesn't have a representation in the underlying simulator (for example from being optimized away). A simple implementation of get_value would be to simply store the Python id of each Bit in a map as mentioned above.
• PythonSimulator could implement get_value with an internal SimBit class that stores the simulated value for a given point in the simulator. When flattening the circuit, the simulator would create an internal map from Bit id's to SimBits, and get_value would simply look in this this map for the appropriate SimBit. This would also allow the python simulator to get rid of the Pipe, since multiple Bits in the circuit can map to one SimBit (for the current implementation, this would mean one SimBit for each input and output of each primitive in the circuit).
• CircuitSimulator would also define a set_inputs function that allows the frontend to set the inputs to the main circuit, and a step function that toggles the clock (if magma aims to support multiple clock domains this would obviously need to be more complicated, perhaps step would advance the simulator forward one "unit" in time).

This leaves the issue of where to store the python simulation functions for circuits in mantle / board primitives. I think most of the solutions aren't 100% ideal since they leak information about the Python simulator into the rest of magma, but I can see this being done by simply having a simulate function added for circuits, or possibly with a special class for circuits that are either primitives or designed to be simulated directly (ie mantle circuits).

from magma.

• An abstract class CircuitSimulator ...

  Sounds good good to me

• The debugger frontend [...] The interface could include methods to query the capabilities of the underlying simulator, since the Python and CoreIR simulators would likely have different limitations.

  This is a great idea. I don't think we need to fully understand what we would query but having the infrastructure for this in place would be great. We could revisit this once we have another simulator to integrate.

• CircuitSimulator would define get_value(bit)...

  Sounds good good to me

• PythonSimulator could implement get_value with an internal SimBit class that stores the simulated value for a given point in the simulator. When flattening the circuit, the simulator would create an internal map from Bit id's to SimBits, and get_value would simply look in this this map for the appropriate SimBit.

  Sounds like a good way to get rid of Pipe while preserving information from the original (unflattened) circuit

• CircuitSimulator would also define a set_inputs function that allows the frontend to set the inputs to the main circuit, and a step function that toggles the clock...

  Sounds good to me. We might want some more sugar on top of these, such as given a set of input vectors (i.e. arguments to set_inputs for N clock cycles), run the circuit N cycles. These convenience functions should become more apparent once we have a couple working examples of using the simulator.

• This leaves the issue of where to store the python simulation functions for circuits in mantle / board primitives. I think most of the solutions aren't 100% ideal since they leak information about the Python simulator into the rest of magma, but I can see this being done by simply having a simulate function added for circuits, or possibly with a special class for circuits that are either primitives or designed to be simulated directly (ie mantle circuits).

  I would think it makes sense to require a simulate function with the primitive definition. Mantle primitives should have this defined and thus any circuit built on top of purely mantle primitives can be simulated. If someone wants to add a board-specific primitive (or for the existing primitives), they will need to define a simulate function. The alternative would be to have the simulator hold the logic to simulate various primitives which would have to be extended with every board addition. I think it's better to put this as a requirement on the Board definition rather than as a part of the simulator implementation.

from magma.

Sounds good to me. We might want some more sugar on top of these, such as given a set of input vectors (i.e. arguments to set_inputs for N clock cycles), run the circuit N cycles...

My thinking was that the frontend could implement these convenience functions itself, on top of the basics provided by the CircuitSimulator, but I could see an argument for more complex functions for efficiency reasons.

I would think it makes sense to require a simulate function with the primitive definition. Mantle primitives should have this defined and thus any circuit built on top of purely mantle primitives can be simulated...

I agree this is probably the way to go, but I'm not sure exactly the best way to do in python. For example the simulator could check if a circuit has this simulate function to determine whether or not it is a primitive, but the current simulator also needs to know whether or not a given primitive has state within it, which would require another attribute added to primitive circuits, which seems a little ugly to just do ad-hoc in the code (and maybe prone to error). I think a cleaner solution would be to add a PrimitiveCircuit class and a DefinePrimitiveCircuit function which accepted the simulator's required attributes and a simulate function. Then the simulator could check if something is isinstance(PrimitiveCircuit). This might be a little over engineered though.

from magma.

By sugar (ala syntax sugar) I meant implement those functions on top of the basic ones your described, so we're on the same page.

PrimitiveCircuit/DefinePrimitiveCircuit sounds like a good way to do it

from magma.

@leonardt I've been working on this and am pretty close to a pull request with CircuitSimulator and PythonSimulator implemented. One question I have is referred to at the top of this post: What should the simulator's entry point be? Is the format for a user defined circuit still the same as it was in 448 (ie the compile method is called on main)?

from magma.

The format for compiling a circuit hasn't changed.

One thing I'd like to consider is that a user might want to define a circuit, simulate it, then compile it to verilog. Ideally this could be done without changing the source file defining the circuit, which motivates a command line option like --simulate.

I still do think a programmatic API should be exposed (e.g. for programmatically invoking magma from a library built on top of it). I believe your implementation from class overloaded the compile function to use a simulated backend. I originally chose simulate instead because I thought it was more explicit (Pythonistas like to be explicit). However, now that I think about it, the above command line option would most likely be implemented by overriding the compile function to target a simulator backend. I'll leave it up to you to decide on what you think is best (we can always modify moving forward, I don't think this is a blocking issue). A better interface design may emerge as we get people actually using the simulator and providing feedback.

from magma.

@leonardt Thanks for the response. One more question: does the simulator need to support primitives that have both combinational and flip flop outputs? For the icestick simulator it didn't, but I'm not sure at what level we want to simulate mantle primitives.

from magma.

Not sure on this one, @phanrahan any thoughts on whether we need to support primitives that have both combinational and registered outputs?

from magma.

@shacklettbp sorry this slipped a bit, just got back from SNAPL. I think for now we can start with the functionality that we need for the icestick simulator.

from magma.

@leonardt I'm still working out a few bugs in the simulator so I decided to separate out my work for flattening circuits (which the simulator depends on), it's in the transforms branch. Should I make a pull request for it? I still need to clean up my tests for it and push those (haven't played with magma's test infrastructure yet).

from magma.

One problem I just found when trying to compile a flattened circuit is that Pat's Makefiles typically assume the top level circuit is named main, but any flattened circuit has _flattened appended to it's name (ie main_flattened). This is because magma caches circuits based on name, so the transformed circuit has to have a different name. Is this worth worrying about? It doesn't matter for the simulator itself since it's only a problem when compiling the verilog.

from magma.

Good catch. How about making main the default, but adding an option to specify the name of the top-level circuit?

from magma.

@phanrahan, Currently I'm not aware of a way to pass information from magma to the makefile used to compile the verilog itself, perhaps I'm misunderstanding what you're saying.

from magma.

@shacklettbp you could patch the Makefile to accept the name of the top level circuit

+ TOP ?= main 

%.blif: %.v
- 	yosys $(QUIET) -p 'synth_ice40 -top main -blif $@' $<
+ 	yosys $(QUIET) -p 'synth_ice40 -top $(TOP) -blif $@' $<

$(TOP) will default to the value main unless specified at the command line (e.g. make TOP=main_flattened)

from magma.

@leonardt, Thanks. On an unrelated note, I was thinking about the entry point for the simulator, and right now the way compile works makes it pretty hard to have something like python circuit.py --simulate without some extra annoying parameters. I was thinking it might be better to have a "frontend" compile function which only accepts the main circuit (for use by user defined circuits), which parses sys.argv itself before passing it onto the backend compile function (something similar to the current compile function). This would allow us to pretty easily add new flags in the future that don't depend on position without modifying every existing magma circuit to pass another sys.argv param into compile. Let me know what you think.

from magma.

@shacklettbp PRs looks good, I'll ask Pat if he has any feedback before merging. Could you write up some documentation on adding support for simulation when defining a primitive? We can add this to the Mantle docs so anyone can easily add simulation support to new or existing primitives as needed.

from magma.

@leonardt Thanks for the reviews, I'll add a little bit of documentation to my mantle PR later this week.

from magma.

@shacklettbp I'm going to close this top-level issue. Any further discussion can tracked in a new issue for the specific topic.

from magma.