Skip to content

Commit

Permalink
Remove unnec. rename in uniq. pass
Browse files Browse the repository at this point in the history
Previously we were calling rename with the same name (i.e. an
unnecessary rename). For normal magma circuits this wasn't an issue,
since the rename was a no-op, but it was surfacing when doing so for
verilog-wrapped circuits, since they can't be renamed.

Added a test for this issue.
  • Loading branch information
Rajsekhar Setaluri committed Dec 12, 2019
1 parent 3b21e41 commit e306c71
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 19 deletions.
19 changes: 1 addition & 18 deletions magma/uniquification.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,27 +56,10 @@ def __call__(self, definition):
else:
if self.mode is not UniquificationMode.UNIQUIFY:
assert self.seen[name][key][0].name == definition.name
else:
type(definition).rename(definition, self.seen[name][key][0].name)
self.seen[name][key].append(definition)

def _run(self, definition):
if not isdefinition(definition):
return

for instance in definition.instances:
instancedefinition = type(instance)
if isdefinition(instancedefinition):
self._run( instancedefinition )

id_ = id(definition)
if id_ in self.definitions:
return
self.definitions[id_] = definition
self(definition)

def run(self):
super(UniquificationPass, self).run()
super().run()
duplicated = []
for name, definitions in self.seen.items():
if len(definitions) > 1:
Expand Down
52 changes: 51 additions & 1 deletion tests/test_uniquify.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,61 @@ def test_hash_verilog():
# Run the uniquification pass as a mechanism to check that foo0 and foo1
# hash to two different things even though they have the same repr.
pass_ = m.UniquificationPass(top, None)
pass_._run(top)
pass_.run()
foo_seen = pass_.seen["foo"]
assert len(foo_seen) == 2
for v in foo_seen.values():
assert len(v) == 1
expected_ids_ = {id(v[0]) for v in foo_seen.values()}
ids_ = {id(foo0), id(foo1)}
assert expected_ids_ == ids_


def test_same_verilog():
"""
This test checks that if we have multiple verilog-wrapped circuits with the
same name and same source, the uniquification pass does *not* try to perform
a rename. As it does not need to.
"""
def _generate_foo():
return m.DefineFromVerilog("""
module foo(input i, output o);
assign o = i;
endmodule""")[0]


class _Cell0(m.Circuit):
IO = ["I", m.In(m.Bit), "O", m.Out(m.Bit)]

@classmethod
def definition(io):
foo = _generate_foo()()
foo.i <= io.I
io.O <= foo.o


class _Cell1(m.Circuit):
IO = ["I", m.In(m.Bit), "O", m.Out(m.Bit)]

@classmethod
def definition(io):
foo = _generate_foo()()
foo.i <= io.I
io.O <= foo.o


class _Top(m.Circuit):
IO = ["I", m.In(m.Bits[2]), "O", m.Out(m.Bits[2])]

@classmethod
def definition(io):
cell0 = _Cell0()
cell1 = _Cell1()
cell0.I <= io.I[0]
cell1.I <= io.I[1]
io.O[0] <= cell0.O
io.O[1] <= cell1.O

# Check that uniq. pass runs successfully.
pass_ = m.UniquificationPass(_Top, None)
pass_.run()

0 comments on commit e306c71

Please sign in to comment.