diff --git a/magma/uniquification.py b/magma/uniquification.py index 15192af680..955cf43c50 100644 --- a/magma/uniquification.py +++ b/magma/uniquification.py @@ -36,8 +36,7 @@ def _hash(definition): class UniquificationPass(DefinitionPass): def __init__(self, main, mode): - super(UniquificationPass, self).__init__(main) - self.definitions = {} + super().__init__(main) self.mode = mode self.seen = {} self.original_names = {} @@ -46,37 +45,22 @@ def __call__(self, definition): name = definition.name key = _hash(definition) - if name not in self.seen: - self.seen[name] = {} - if key not in self.seen[name]: - if self.mode is UniquificationMode.UNIQUIFY and len(self.seen[name]) > 0: - new_name = name + "_unq" + str(len(self.seen[name])) + seen = self.seen.setdefault(name, {}) + if key not in seen: + if self.mode is UniquificationMode.UNIQUIFY and len(seen) > 0: + new_name = name + "_unq" + str(len(seen)) type(definition).rename(definition, new_name) - self.seen[name][key] = [definition] + seen[key] = [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) + assert seen[key][0].name == name + elif name != seen[key][0].name: + new_name = seen[key][0].name + type(definition).rename(definition, new_name) + seen[key].append(definition) def run(self): - super(UniquificationPass, self).run() + super().run() duplicated = [] for name, definitions in self.seen.items(): if len(definitions) > 1: diff --git a/tests/gold/uniquify_multiple_rename.json b/tests/gold/uniquify_multiple_rename.json new file mode 100644 index 0000000000..cae66fb585 --- /dev/null +++ b/tests/gold/uniquify_multiple_rename.json @@ -0,0 +1,55 @@ +{"top":"global._Top", +"namespaces":{ + "global":{ + "modules":{ + "Foo":{ + "type":["Record",[ + ["I",["Array",2,"BitIn"]], + ["O",["Array",2,"Bit"]] + ]], + "connections":[ + ["self.O","self.I"] + ] + }, + "Foo_unq1":{ + "type":["Record",[ + ["I",["Array",3,"BitIn"]], + ["O",["Array",3,"Bit"]] + ]], + "connections":[ + ["self.O","self.I"] + ] + }, + "_Top":{ + "type":["Record",[ + ["I0",["Array",2,"BitIn"]], + ["I1",["Array",3,"BitIn"]], + ["I2",["Array",3,"BitIn"]], + ["O0",["Array",2,"Bit"]], + ["O1",["Array",3,"Bit"]], + ["O2",["Array",3,"Bit"]] + ]], + "instances":{ + "Foo_inst0":{ + "modref":"global.Foo" + }, + "Foo_inst1":{ + "modref":"global.Foo_unq1" + }, + "Foo_inst2":{ + "modref":"global.Foo_unq1" + } + }, + "connections":[ + ["self.I0","Foo_inst0.I"], + ["self.O0","Foo_inst0.O"], + ["self.I1","Foo_inst1.I"], + ["self.O1","Foo_inst1.O"], + ["self.I2","Foo_inst2.I"], + ["self.O2","Foo_inst2.O"] + ] + } + } + } +} +} diff --git a/tests/test_uniquify.py b/tests/test_uniquify.py index 39a04e9b6f..9aad5e9140 100644 --- a/tests/test_uniquify.py +++ b/tests/test_uniquify.py @@ -4,7 +4,7 @@ def test_verilog_field_uniquify(): - # https://github.com/phanrahan/magma/issues/330 + # https://github.com/phanrahan/magma/issues/330. HalfAdder = m.DefineCircuit('HalfAdder', 'A', m.In(m.Bit), 'B', m.In(m.Bit), 'S', m.Out(m.Bit), 'C', m.Out(m.Bit)) @@ -14,6 +14,7 @@ def test_verilog_field_uniquify(): ''' m.EndCircuit() + def test_uniquify_equal(): foo = m.DefineCircuit("foo", "I", m.In(m.Bit), "O", m.Out(m.Bit)) m.wire(foo.I, foo.O) @@ -173,7 +174,7 @@ 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(): @@ -181,3 +182,100 @@ def test_hash_verilog(): 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() + + +def test_multiple_renamed(): + def _gen_foo(width): + class Foo(m.Circuit): + IO = ["I", m.In(m.Bits[width]), "O", m.Out(m.Bits[width])] + + @classmethod + def definition(io): + io. O <= io.I + + return Foo + + Foo0 = _gen_foo(2) + Foo1 = _gen_foo(3) + Foo2 = _gen_foo(3) + + class _Top(m.Circuit): + IO = [ + "I0", m.In(m.Bits[2]), + "I1", m.In(m.Bits[3]), + "I2", m.In(m.Bits[3]), + "O0", m.Out(m.Bits[2]), + "O1", m.Out(m.Bits[3]), + "O2", m.Out(m.Bits[3]), + ] + + @classmethod + def definition(io): + foo0 = Foo0() + foo1 = Foo1() + foo2 = Foo2() + + foo0.I <= io.I0 + io.O0 <= foo0.O + + foo1.I <= io.I1 + io.O1 <= foo1.O + + foo2.I <= io.I2 + io.O2 <= foo2.O + + BASENAME = "uniquify_multiple_rename" + m.compile(f"build/{BASENAME}", _Top, output="coreir") + assert check_files_equal(__file__, + f"build/{BASENAME}.json", + f"gold/{BASENAME}.json")