magma
magma copied to clipboard
phanrahan
Enforce Unique Names
I suggest we change magma to enforce that module names are unique at definition time. Currently, if the user forgets to memoize a generator, their code will create thousands of duplicate module definitions, which the compiler will silently accept. This is causing us to have hour long compile times, and it's not possible to debug this if we don't raise an error.
I added a quick fix to enforce this behavior, but (if we agree this is the behavior we want), I will need help squashing all the duplicate name bugs. It looks like a large number of generators in the repo that are not being memoized.
Also @leonardt there was a breaking change to the combinational file on my machine which I fixed (?) here.
This issue has ping-ponged a bit in magma previously. I think this condition used to be enforced, but we changed it because not enforcing it was more general and robust. In particular, if you have a top level design that's using components from disparate places, they may not have unique names. And in fact they may not be modifiable (imagine you're importing some third party magma code or something).
Plus, the issue of not caching generators is separate and should be solved "directly". I think one reasonable thing would be to make auto-uniquification off by default in the compiler; i.e. this error is raised at compile time rather than define time. This mode already exists, it just has to be enabled.
@rsetaluri I think that would help with performance (avoiding the repr uniquification time), another incremental option might be to allow selective whitelist of names to uniquify (so if there's a commonly used name such as Util or something, uniquification can be enabled for those circuits, while off by default.
Another improvement would be to add automatic name generation in the Generator class, right now it caches so it avoids regenerating multiple instances, but I think if the use doesn't provide a name to the generated Circuit, it will still trigger uniquification downstream for each instance of the generator