Propagate definition renames to instances

[rsetaluri]

Right now, if and when we perform renames for uniquification, the new name is not propagated to instances (it only affects the definition). First this can impact correctness, because if we have different circuits with the same name, instance names may collide if they do not include the proper _unqN suffix. Secondly, it becomes hard to map definitions to instances if they do not have the same name in the verilog.

Finally, if we want to support #332 we will also need to relate definitions to instances.

The important question here is performance related: is it feasible to store all instances of a defn, and perform some operation on all of them (such as renaming) as those operations happen on the defn?

[rsetaluri]

TODO: write a test to show why this can lead to incorrect results.

[leonardt]

An alternative to storing all instances would be to make the name lookup in the instance lazy (e.g. look up the __name__ field of the class). Would this also address the issue? This incurs a latency cost, but I don't know how often we're actually doing the lookup.

The tradeoff seems to be memory vs latency. I don't think the latency of another level of indirection in the name lookup should be that costly, seeing as Python already has multiple levels of indirection under the hood.

We could consider the complexity of each solution w.r.t. to a design. The space complexity of storing instances will scale linearly with the number of circuit definitions and instances (n * m with n circuits and m instances of each circuit). The latency cost of a dynamic name lookup would scale linearly with the total number of instances (n * m lookups with an extra level of indirection, assuming we lookup the name once during compilation EDIT this should've been n * m not m).

Also, I went to check in the coreir backend, it seems like it should be already dynamically looking up the name for the circuit in one case, see https://github.com/phanrahan/magma/blob/f5e8fea15f3318ab397d26ce749554337257cf53/magma/backend/coreir_.py#L188 I'm not sure if that's always the case though, so it would be worth digging a bit more into exactly where the issue is arising (test case will help). Also, there might be something subtle going on related to renaming and metaclasses that's causing it not to propogate.

[leonardt]

I was curious whether setting __name__ was doing what we thought, and it seems like it does

❯ python
Python 3.7.0 (default, Jun 28 2018, 07:39:16)
[Clang 4.0.1 (tags/RELEASE_401/final)] :: Anaconda, Inc. on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class A:
...     pass
...
>>> a = A()
>>> a.__class__.__name__
'A'
>>> A.__name__ = "B"
>>> a.__class__.__name__
'B'
>>>

[leonardt]

Also, some preliminary research reveals that if we plan to store the instances, we should use weakref so it doesn't prevent the instances from being garbage collected (see https://stackoverflow.com/questions/328851/printing-all-instances-of-a-class). This is mainly an issue for cases where instances are removed.