Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Mantle and FPGAs #1300

Open
leonardt opened this issue Aug 8, 2023 · 13 comments
Open

[RFC] Mantle and FPGAs #1300

leonardt opened this issue Aug 8, 2023 · 13 comments

Comments

@leonardt
Copy link
Collaborator

leonardt commented Aug 8, 2023

While we're migrating mantle into the core, we have an opportunity to revisit our design for the FPGA specific mantle targets.

Primitives

Magma/mantle offer a set of primitives that can be used to construct higher level circuits. In the magma core, we use generic implementations of the primitive, e.g. the + operators translates to a + in mlir/verilog. In mantle, we also have a suite of FPGA specific implementations of these primitives, that are constructed bottom up using the FPGA primitives* (e.g. LUTS, PLBs). These implementations may leverage specific qualities of the architecture to optimize their implementations, such as the LUT size and carry logic.

Generic Mantle Circuits

More broadly in mantle, we have a suite of generic circuits that reference the magma primitives. For example, a counter uses a Register and Adder, which can be mapped to a generic implementation or an FPGA specific implementation depending on the context. The key design issue here is how to architect the magma/mantle package so that the generic circuits can be written in a generic style (refer to these abstract primitives) and then a specific implementation can be selected later depending on the context.

Possible Solutions

  • The current mantle approach is to use dynamic import behavior (https://github.com/phanrahan/mantle/blob/master/mantle/__init__.py). The user selects a mantle_target before importing mantle, then mantle populates its namespace with specific implementations depending on the choice.
  • Use generic primitives by default, replace with specific implementations using a pass.
  • Overwrite implementations using monkey patching (e.g. change the __add__ operator at runtime to use a specified implementation)

Issues to consider

  • Dynamic import behavior seems like something that should be avoided, I was never happy with the solution and it seemed prone to errors and bugs, while not being ergonomic for the user. What if part of the design wants to use generic implementations while others want to use platform specific? There's no way to modify the target after its been imported initially.
  • Laziness (replacing later with a pass) could prevent introspection that is aware of the platform specific implementation. However this issue could be worked around by giving the user a hook to "elaborate" a sub-hierarchy by running the pass early.
  • Monkey patching operators at runtime also seems potentially unsafe, but is also one of the "features" of Python

@phanrahan I know this is an issue we spent quite some time discussing in the past, I'll see if I can resurrect any useful notes but perhaps you remember any issues I'm forgetting.

@phanrahan
Copy link
Owner

I think we should rely on generic implementations as much as possible by default. I know I have used a verilog target in the past for FPGAs without problem. That should work with MLIR.

We need access to FPGA primitives such as SB_LUT4. We need a mechanism to declare built-in circuits. Besides FPGAs there are built-in verilog primitives.

The existing mantle implementation is overly complicated. We had common generators which worked with primitives for different FPGAs (e.g. spartan3 and ice40). We needed dependency injection to inject a back-end with its associated primitives. I suggest we discuss how to do this best in person. It's not a high priority for me right now.

@leonardt
Copy link
Collaborator Author

leonardt commented Aug 8, 2023

Another related issue: in mantle we have mantle40 which is a suite of higher level circuits (i.e. have generic counterparts) that have platform specific implementations. For example, the decode and mux logic can be implemented compactly using LUTs. This is perhaps more akin to importing a platform specific library such as "CUBLAS" versus Intel MKL

@phanrahan
Copy link
Owner

I think the main complexity was using the mantle circuits for built-in operations like addition that we implemented using operator overloading in magma. It was not as much of a problem for non-built-in components like counters and decoders
which had to be wired up like other modules.

Let's go with verilog for operator overloading. And retain the ability to explicitly instantiate mantle circuits and wire them up.

@phanrahan
Copy link
Owner

phanrahan commented Aug 10, 2023

I am porting my library.

In SB_PLL_CORE

I need to pass in parameters such as the following

        params = {}
        params["FEEDBACK_PATH"] = "SIMPLE"
        params["PLLOUT_SELECT"] = "GENCLK"
        # Reference clock divider (div+1) [0, ..., 15]
        params["DIVR"] = (divr, 4) 
        # Feedback divider (div+1) [0, ..., 63]
        params["DIVF"] = (divf, 7)
        # VCO divider (divq+1) [0, ..., 6]
        params["DIVQ"] = (divq, 3)
        params["FILTER_RANGE"] = (filter, 3)

        pll = SB_PLL40_CORE(**params)

How do i declare this in param_types?

Note that (divr, 4) means that divr should be a 4-bit value. I propose to use

params["DIVR"] = ht.BitVector[4](divr)

Here is another example of parameter passing.

SB_RAM40_4K #(
    .INIT_0(256'h0000000000000000000000000000000000000000000000000000000000ff0001),
    .INIT_1(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_2(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_3(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_4(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_5(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_6(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_7(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_8(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_9(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_A(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_B(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_C(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_D(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_E(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .INIT_F(256'h0000000000000000000000000000000000000000000000000000000000000000),
    .READ_MODE(0),
    .WRITE_MODE(0)
) SB_RAM40_4K_inst0 ( .. );

@phanrahan
Copy link
Owner

Another issue I had was passing in additional arguments to primitive circuits. For example, the DeclareCircuit for SB_PLL_CORE was

SB_PLL40_CORE = DeclareCircuit('SB_PLL40_CORE',
            "REFERENCECLK", In(Clock),
            "RESETB", In(Bit),
            "BYPASS", In(Bit),
            "PLLOUTCORE", Out(Bit),
            "PLLOUTGLOBAL", Out(Clock),
            coreir_lib='ice40')

How are arguments like coreir_lib handled?

@phanrahan
Copy link
Owner

In terms of testing, I can test the generated verilog against the gold.

There are also simulation functions.

@phanrahan
Copy link
Owner

phanrahan commented Aug 10, 2023

The gate-level verilog functions use unnamed positional arguments.

How do I declare these circuits using class Circuit?

Not = DeclareCircuit('not', '', Out(Bit), '', In(Bit))
Buf = DeclareCircuit('buf', '', Out(Bit), '', In(Bit))

@leonardt
Copy link
Collaborator Author

@phanrahan just pushed this commit: 91ad707

Which adds support for string parameters (and also adds the PLL definition)

(Note, @rsetaluri may be interested) This was a case where I need to wrap a circuit with parameters for the instantiation (here we compute the PLL params based off higher level freqin/out params). Now this doesn't necessarily need a Generator because we're not actually generating a new type, instead we're just wrapping the instantiation of the PLL.

Rather than use Generator here, it might make more sense to use a simple function def with a make_* prefix?

@leonardt
Copy link
Collaborator Author

Also, @rsetaluri do you know if MLIR can handle unnammed arguments like above? Otherwise may need to use inline_verilog and a wrapper

@phanrahan
Copy link
Owner

phanrahan commented Aug 12, 2023

Thanks! I pulled it and it looks good.

I don't think it should be wrapped in a generator. For example, SB_LUT has parameters and each LUT with different parameters are different instances, not different classes.

Also, the _make_pll_io helper function seems unnecessary.

@cdonovick
Copy link
Collaborator

One solution that has always made sense to me for bitvector operators was to simply create a new bv type for each backend, e.g., LaticeVector or AlteraVector. Where each type instantiates the appropriate primitives for it operators. One could then define things using a sort of PEak style where things are defined as generator which takes a BV type as an argument to the generator e.g.:

def make_module(bv_t):
    class Module(Circuit): ...
    
# or
class ModuleGen(Generator2):
    def __init__(self, bv_t): ...

While choosing a backend upfront is a bit unusual its not really different from the Mantle style of choosing a mantle target before importing mantle. Now, of course this requires a user of the generated module to then decide on their backend at instantiation time or to propagate this pattern e.g.:

def make_wrapper(bv_t):
    Module = make_module(bv_t)
    ...

This doesn't need to be simply limited to bv types this could be extended to any set of modules:

def make_module(bv_t, mux_t, mem_t, ...): ...

Now if this seems like a reasonable direction leveraging some of PEak (or just lifting some its design patterns) might make sense, as making the above pattern ergonomic is one the core functions of PEak.

@phanrahan
Copy link
Owner

I like that idea. But I am don't understand all the issues like you do. Sorry, to be so out of touch.

@phanrahan
Copy link
Owner

phanrahan commented Aug 18, 2023

In terms of my issues, most of them have to do with the low-level MLIR interface. For example, positional arguments not keyword arguments. Will different backend types solve these problems?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants