Prototype linked definitions flow

[leonardt]

Adds capability to generate multiple variants of a definitions using `ifdef in verilog to select.

Adds a JSON field that is a map from key (ifdef variable) to module (containing alternate definition). Adds IR API to represent map from key to module Adds verilog code generation logic to generate code for alternate definitions (normally not in the instance graph) as well as inlining them into the call site (to avoid extra hierarchy). Since verilogAST-cpp doesn't support cloning structural statements, for now we just store the module body as a string, but in the future we should add cloning logic so we can copy the AST. Since this happens at the end of module code generation (all verilogAST passes have been run), I'm actually not sure if it matters so much that we just inline the string.

[rsetaluri]

I think this looks good. I have a few outsanding concerns.

1. Error reporting. Not super high stakes, but while we're here, we should some logging/diagnostics, especially if we link the same key or something. Mostly just a reminder for myself to add this.

2. More real question. There are I think 3 ways to define the link_module interface:

   1. You can only call link_module on a decl, i.e. link_module :: decl -> key -> defn. If a decl has 0 modules linked it stays an external module, if it has 1 module linked, that becomes the default definition (i.e. there is no ifdef business, just a normal module). If it has > 1 module linked, then we do the ifdef based switching on the key; additionally there should be a way to specify if there is a default definition (it's possible but unlikely you would not have a default, but it kind of falls out anyway).
   2. You can only call link_module on a defn, i.e. link_module :: defn -> key -> defn. The existing definition is always treated as a default, and for the rest, the key is used as a switch. If there are no linked modules, then the behavior is as before.
   3. You can call link_module on either a defn or decl, i.e. link_module :: (defn | decl) -> key -> defn. This would basically be a join of the 2 previous conditions; if it's decl do (i), if it's a defn do (ii).

   I think either (i) or (iii) is best here. In particular thinking about our current use case, we only want to do the linking for a subset of the original instances of this type. In the case of (ii) we would have to replace all those instances first with a copy of the original module, then link in the secondary case - so as to not do the linking for all instances of that type. Also, I think it's easier to go the other way (try to achieve (ii) with the mechanism of (i)/(iii) than the other way around).

3. How do we ensure dependency analysis works correctly here? This is not just an academic question because if we have a module definition which is only used as a linked_module, then it won't get picked up as part of dependency analysis for compilation. I guess a similar question can be asked for how to do passes in the presence of linking.
4. Derived from (3): how can we make sure we don't have any circular dependencies through linking? (Also disallowing linking to self, which is just a special case of this.)

@leonardt let me know what you think. Happy to give more rationale offline.

PS. The more I think about it, I think (3) is a pretty fundamental question. I.e. I think this kind of feature breaks the abstraction that we can analyze a circuit at our "compile" time, i.e. there are no ifdef like things. I almost think of this as an "unsafe" feature for that reason: a feature that can be used to get some precise code generated, but one that might break some guarantees (similar to inline_verilog).

The alternative technique would be to instance all the linked definitions then generate special "muxes" that are just ifdef switches on the outputs. This could either be in magma or CoreIR. That way we get the full dependencies still. Also, symbol table stuff will work through this path, whereas it might not with the other way.

[leonardt]

(ii) we would have to replace all those instances first with a copy of the original module, then link in the secondary case - so as to not do the linking for all instances of that type

I think this makes sense, basically instead of "copying" the module, we create a new decl and point to the original defn as the default.

The options seem to be:

1. have a special key be default, e.g. DEFAULT
2. have a special API for adding the default (which stores it in a special field rather than in the dictionary)

How do we ensure dependency analysis works correctly here

At least in the case of verilog code generation, it seems we need the instance graph pass to be aware of multiple definitions (linked) and add them to the set of modules traversed. Not sure about other passes.

Derived from (3): how can we make sure we don't have any circular dependencies through linking? (Also disallowing linking to self, which is just a special case of this.)

Seems like we could do a graph traversal analysis pass to check this?

[rsetaluri]

Re: the first point, I'm not too opinionated. Just want to avoid kind of having a bunch of different cases and different code paths, depending on default etc. I think a separate field + a link_default kind of API would be most explicit and clear.

Seems like we could do a graph traversal analysis pass to check this?

Yeah I agree. Think this problem comes down to modifying the base traversal passes to go over linked definitions.

[leonardt]

I've updated the instancegraph pass to support the linked definitions (so we no longer have to add extra logic to the verilog backend to handle the dependencies).

I've adjusted the API to support adding a default definition.

I've updated the logic to support default definition, as well as only one linked definition, as well as two or more linked definitions with no default.

[rsetaluri]

Just making a note here that right now we output Verilog for linked definitions which are not otherwise instanced because they are collected in the instance graph pass. Ideally we would cull those from the output, but it's not a major issue.

[leonardt]

Looks like we can't use Travis anymore since we ran out of build credits, we should move this repo to github workflows (cc @rdaly525 )

[rdaly525]

Agreed that we should move to github workflows. In the mean time I put in a request for more travis build time which usually works within a day.

[rdaly525]

@leonardt, there are more travis credits now if you want to add a commit to trigger the build.

[leonardt]

Thanks!