Merge 7aa2e1f into 4021009

magma/__init__.py

@@ -63,6 +63,8 @@ def set_mantle_target(t):
 from .is_primitive import isprimitive
 from .is_definition import isdefinition
 
+from .uniquification import UniquificationPass
+
 from .product import Product
 
 

magma/backend/coreir_.py

@@ -304,10 +304,15 @@ def compile_definition(self, definition):
 
         # If this module was imported from verilog, do not go through the
         # general module construction flow. Instead just attach the verilog
-        # source as metadata and return the module.
+        # source as metadata and return the module. Also, we attach any
+        # contained instances as CoreIR instances.
         if hasattr(definition, "verilogFile") and definition.verilogFile:
             verilog_metadata = {"verilog_string": definition.verilogFile}
             coreir_module.add_metadata("verilog", json.dumps(verilog_metadata))
+            module_definition = coreir_module.new_definition()
+            coreir_module.definition = module_definition
+            for instance in definition.instances:
+                self.compile_instance(instance, module_definition)
             return coreir_module
 
         module_definition = coreir_module.new_definition()

magma/circuit.py

@@ -139,6 +139,9 @@ def _repr_html_(cls):
         return circuit_to_html(cls)
 
     def rename(cls, new_name):
+        if cls.verilogFile:
+            raise Exception("Can not rename a verilog wrapped file")
+
         old_name = cls.name
         cls.name = new_name
         cls.coreir_name = new_name
@@ -151,6 +154,10 @@ def rename(cls, new_name):
         # <new_name>" existing anywhere else, most likely in comments etc. The
         # more robust way to do this would to modify the AST directly and
         # generate the new verilog code.
+        #
+        # Instead, at the top of this function, we raise an exception for
+        # verilog wrapped files. That is, for now it is considered an error to
+        # try to rename a verilog wrapped circuit.
         if cls.verilogFile:
             find_str = f"module {old_name}"
             replace_str = f"module {new_name}"

magma/fromverilog.py

@@ -16,6 +16,7 @@
 
 logger = logging.getLogger('magma').getChild('from_verilog')
 
+
 __all__  = ['DeclareFromVerilog']
 __all__ += ['DeclareFromVerilogFile']
 __all__ += ['DeclareFromTemplatedVerilog']
@@ -28,11 +29,30 @@
 
 class ModuleVisitor(NodeVisitor):
     def __init__(self):
-        self.nodes = []
+        self.defns = {}
+        self.__defn_stack = []
+        self.__instances = {}
+
+    def visit_ModuleDef(self, defn):
+        if defn.name in self.defns:
+            raise Exception(f"Defn with name {defn.name} appears twice")
+        self.defns[defn.name] = defn
+        # Collect instances in this definition.
+        self.__instances[defn] = set()
+        self.__defn_stack.append(defn)
+        self.generic_visit(defn)
+        self.__defn_stack.pop()
+        return defn
+
+    def visit_Instance(self, instance):
+        defn = self.__defn_stack[-1]
+        assert instance not in self.__instances[defn]
+        self.__instances[defn].add(instance)
+        return instance
+
+    def get_instances(self, defn):
+        return self.__instances[defn]
 
-    def visit_ModuleDef(self, node):
-        self.nodes.append(node)
-        return node
 
 def convert(input_type, target_type):
     if isinstance(input_type, _BitKind) and \
@@ -111,48 +131,50 @@ def ParseVerilogModule(node, type_map):
 
     return node.name, args
 
-def FromVerilog(source, func, type_map, target_modules=None):
-    parser = VerilogParser()
 
+def FromVerilog(source, func, type_map, target_modules=None, shallow=False):
+    parser = VerilogParser()
     ast = parser.parse(source)
-    #ast.show()
-
-    v = ModuleVisitor()
-    v.visit(ast)
-
-    if func == DefineCircuit:
-        # Only allow a single verilog module unless we're only defining one
-        # circuit (only one module in target_modules), otherwise, they would all
-        # use the same source, so if they are compiled together, there will be
-        # multiple definitions of the same verilog module.
-        assert len(v.nodes) == 1 or (target_modules and len(target_modules) == 1)
-    modules = []
-    for node in v.nodes:
-        if target_modules is not None and node.name not in target_modules:
-            continue
-        try:
-            name, args = ParseVerilogModule(node, type_map)
-            circuit = func(name, *args)
-            if func == DefineCircuit:
-                # inline source
-                circuit.verilogFile = source
-                EndDefine()
-            circuit.verilog_source = source
-            modules.append(circuit)
-        except Exception as e:
-            logger.warning(f"Could not parse module {node.name} ({e}), "
-                           f"skipping")
-    if not modules:
+    visitor = ModuleVisitor()
+    visitor.visit(ast)
+
+    def _get_lines(start_line, end_line):
+        if shallow:
+            return source
+        lines = source.split("\n")
+        return "\n".join(lines[start_line - 1:end_line])
+
+    magma_defns = {}
+    for name, verilog_defn in visitor.defns.items():
+        parsed_name, args = ParseVerilogModule(verilog_defn, type_map)
+        assert parsed_name == name
+        magma_defn = func(name, *args)
+        if func == DefineCircuit:
+            # Attach relevant lines of verilog source.
+            magma_defn.verilogFile = _get_lines(
+                verilog_defn.lineno, verilog_defn.end_lineno)
+            if not shallow:
+                for instance in visitor.get_instances(verilog_defn):
+                    instance_defn = magma_defns[instance.module]
+                    instance_defn()
+            EndDefine()
+        magma_defn.verilog_source = source
+        magma_defns[name] = magma_defn
+
+    if len(magma_defns) == 0:
         logger.warning(f"Did not import any modules from verilog, either could "
                        f"not parse or could not find any of the target_modules "
                        f"({target_modules})")
-    return modules
+    if target_modules is None:
+        return list(magma_defns.values())
+    # Filter modules based on target_modules list.
+    return [v for k, v in magma_defns.items() if k in target_modules]
 
-def FromVerilogFile(file, func, type_map, target_modules=None):
+def FromVerilogFile(file, func, type_map, target_modules=None, shallow=False):
     if file is None:
         return None
     verilog = open(file).read()
-    result = FromVerilog(verilog, func, type_map, target_modules)
+    result = FromVerilog(verilog, func, type_map, target_modules, shallow)
     # Store the original verilog file name, currently used by m.compile to
     # generate a .sv when compiling a circuit that was defined from a verilog
     # file
@@ -184,15 +206,16 @@ def DeclareFromTemplatedVerilogFile(file, type_map={}, **kwargs):
     return FromTemplatedVerilogFile(file, DeclareCircuit, type_map, **kwargs)
 
 
-def DefineFromVerilog(source, type_map={}, target_modules=None):
-    return FromVerilog(source, DefineCircuit, type_map, target_modules)
+def DefineFromVerilog(source, type_map={}, target_modules=None, shallow=False):
+    return FromVerilog(source, DefineCircuit, type_map, target_modules,
+                       shallow=shallow)
 
-def DefineFromVerilogFile(file, target_modules=None, type_map={}):
-    return FromVerilogFile(file, DefineCircuit, type_map, target_modules)
+def DefineFromVerilogFile(file, target_modules=None, type_map={}, shallow=False):
+    return FromVerilogFile(file, DefineCircuit, type_map, target_modules,
+                           shallow=shallow)
 
 def DefineFromTemplatedVerilog(source, type_map={}, **kwargs):
     return FromTemplatedVerilog(source, DefineCircuit, type_map, **kwargs)
 
 def DefineFromTemplatedVerilogFile(file, type_map={}, **kwargs):
     return FromTemplatedVerilogFile(file, DefineCircuit, type_map, **kwargs)
-

magma/uniquification.py

@@ -1,3 +1,4 @@
+from dataclasses import dataclass
 from enum import Enum, auto
 from .is_definition import isdefinition
 from .logging import warning, error
@@ -14,6 +15,25 @@ class UniquificationMode(Enum):
     UNIQUIFY = auto()
 
 
+@dataclass(frozen=True)
+class _HashStruct:
+    defn_repr: str
+    is_verilog: bool
+    verilog_str: bool
+
+
+def _make_hash_struct(definition):
+    repr_ = repr(definition)
+    if hasattr(definition, "verilogFile") and definition.verilogFile:
+        return _HashStruct(repr_, True, definition.verilogFile)
+    return _HashStruct(repr_, False, "")
+
+
+def _hash(definition):
+    hash_struct = _make_hash_struct(definition)
+    return hash(hash_struct)
+
+
 class UniquificationPass(DefinitionPass):
     def __init__(self, main, mode):
         super(UniquificationPass, self).__init__(main)
@@ -24,7 +44,7 @@ def __init__(self, main, mode):
 
     def __call__(self, definition):
         name = definition.name
-        key = hash(repr(definition))
+        key = _hash(definition)
 
         if name not in self.seen:
             self.seen[name] = {}

tests/test_uniquify.py

@@ -1,3 +1,4 @@
+import pytest
 import magma as m
 from magma.testing import check_files_equal
 
@@ -123,3 +124,60 @@ def DeclareCoreirCircuit(*args, **kwargs):
     assert check_files_equal(__file__,
                              f"build/{top.name}.json",
                              "gold/uniquification_key_error_mux.json")
+
+
+def test_uniquify_verilog():
+    [foo0] = m.DefineFromVerilog("""
+module foo(input I, output O);
+    assign O = I;
+endmodule""")
+    [foo1] = m.DefineFromVerilog("""
+module foo(input II, output OO);
+    assign OO = II;
+endmodule""")
+    assert repr(foo0) != repr(foo1)
+    top = m.DefineCircuit("top", "I", m.In(m.Bit), "O", m.Out(m.Bit))
+    foo0_inst = foo0()
+    foo1_inst = foo1()
+    m.wire(top.I, foo0_inst.I)
+    m.wire(foo0_inst.O, foo1_inst.II)
+    m.wire(foo1_inst.OO, top.O)
+    m.EndDefine()
+
+    with pytest.raises(Exception) as pytest_e:
+        m.compile(f"top", top, output="coreir")
+        assert False
+    assert pytest_e.type is Exception
+    assert pytest_e.value.args == \
+        ("Can not rename a verilog wrapped file",)
+
+
+def test_hash_verilog():
+    [foo0] = m.DefineFromVerilog("""
+module foo(input I, output O);
+    assign O = I;
+endmodule""")
+    foo1 = m.DefineCircuit("foo", "I", m.In(m.Bit), "O", m.Out(m.Bit))
+    m.EndCircuit()
+
+    top = m.DefineCircuit("top", "I", m.In(m.Bit), "O", m.Out(m.Bit))
+    foo0_inst = foo0()
+    foo1_inst = foo1()
+    m.wire(top.I, foo0_inst.I)
+    m.wire(foo0_inst.O, foo1_inst.I)
+    m.wire(foo1_inst.O, top.O)
+    m.EndDefine()
+
+    assert repr(foo0) == repr(foo1)
+
+    # 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)
+    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_

tests/test_verilog/build/test_rxmod.v

@@ -50,4 +50,3 @@ begin
   end
 end
 endmodule
-

tests/test_verilog/gold/test_rxmod.v

@@ -50,4 +50,3 @@ begin
   end
 end
 endmodule
-

tests/test_verilog/gold/test_rxmod_top.json

@@ -9,7 +9,7 @@
           ["data",["Array",8,"Bit"]],
           ["valid","Bit"]
         ]],
-        "metadata":{"verilog":{"verilog_string":"module RXMOD(\n  input RX, \n  input CLK,\n  output [7:0] data,\n  output valid);\n\nreg RX_1;\nreg RX_2;\nalways @(posedge CLK) begin\n  RX_1 <= RX;\n  RX_2 <= RX_1;\nend\n\nwire RXi;\nassign RXi = RX_2;\n\nreg [8:0] dataReg;\nreg validReg = 0;\nassign data = dataReg[7:0];\nassign valid = validReg;\n\nreg [12:0] readClock = 0; // which subclock?\nreg [3:0] readBit = 0; // which bit? (0-8)\nreg reading = 0;\n\n\nalways @ (posedge CLK)\nbegin\n  if(RXi==0 && reading==0) begin\n    reading <= 1;\n    readClock <= 150; // sample to middle of second byte\n    readBit <= 0;\n    validReg <= 0;\n  end else if(reading==1 && readClock==0 && readBit==8) begin\n    // we're done\n    reading <= 0;\n    dataReg[8] <= RXi;\n    validReg <= 1;\n  end else if(reading==1 && readClock==0) begin\n    // read a byte\n    dataReg[readBit] <= RXi;\n    readClock <= 100;\n    readBit <= readBit + 1;\n    validReg <= 0;\n  end else if(reading==1 && readClock>0) begin\n    readClock <= readClock - 1;\n    validReg <= 0;\n  end else begin\n    validReg <= 0;\n  end\nend\nendmodule\n"}}
+        "metadata":{"verilog":{"verilog_string":"module RXMOD(\n  input RX, \n  input CLK,\n  output [7:0] data,\n  output valid);\n\nreg RX_1;\nreg RX_2;\nalways @(posedge CLK) begin\n  RX_1 <= RX;\n  RX_2 <= RX_1;\nend\n\nwire RXi;\nassign RXi = RX_2;\n\nreg [8:0] dataReg;\nreg validReg = 0;\nassign data = dataReg[7:0];\nassign valid = validReg;\n\nreg [12:0] readClock = 0; // which subclock?\nreg [3:0] readBit = 0; // which bit? (0-8)\nreg reading = 0;\n\n\nalways @ (posedge CLK)\nbegin\n  if(RXi==0 && reading==0) begin\n    reading <= 1;\n    readClock <= 150; // sample to middle of second byte\n    readBit <= 0;\n    validReg <= 0;\n  end else if(reading==1 && readClock==0 && readBit==8) begin\n    // we're done\n    reading <= 0;\n    dataReg[8] <= RXi;\n    validReg <= 1;\n  end else if(reading==1 && readClock==0) begin\n    // read a byte\n    dataReg[readBit] <= RXi;\n    readClock <= 100;\n    readBit <= readBit + 1;\n    validReg <= 0;\n  end else if(reading==1 && readClock>0) begin\n    readClock <= readClock - 1;\n    validReg <= 0;\n  end else begin\n    validReg <= 0;\n  end\nend\nendmodule"}}
       },
       "top":{
         "type":["Record",[

tests/test_verilog/gold/test_verilog_dependency_top.json

@@ -0,0 +1,42 @@
+{"top":"global.top",
+"namespaces":{
+  "global":{
+    "modules":{
+      "bar":{
+        "type":["Record",[
+          ["I","BitIn"],
+          ["O","Bit"]
+        ]],
+        "instances":{
+          "foo_inst0":{
+            "modref":"global.foo"
+          }
+        },
+        "metadata":{"verilog":{"verilog_string":"module bar(input I, output O);\n    foo foo_inst(I, O);\nendmodule"}}
+      },
+      "foo":{
+        "type":["Record",[
+          ["I","BitIn"],
+          ["O","Bit"]
+        ]],
+        "metadata":{"verilog":{"verilog_string":"module foo(input I, output O);\n    assign O = I;\nendmodule"}}
+      },
+      "top":{
+        "type":["Record",[
+          ["I","BitIn"],
+          ["O","Bit"]
+        ]],
+        "instances":{
+          "bar_inst0":{
+            "modref":"global.bar"
+          }
+        },
+        "connections":[
+          ["self.I","bar_inst0.I"],
+          ["self.O","bar_inst0.O"]
+        ]
+      }
+    }
+  }
+}
+}

tests/test_verilog/test_from_file.py

@@ -76,7 +76,7 @@ def test_decl_list():
 
 def test_from_sv():
     file_path = os.path.dirname(__file__)
-    test_pe = m.DefineFromVerilogFile(os.path.join(file_path, "test_pe.sv"))[0]
+    test_pe = m.DefineFromVerilogFile(os.path.join(file_path, "test_pe.sv"), shallow=True)[0]
 
 
     if os.path.exists("build/test_pe.sv"):
@@ -92,3 +92,23 @@ def test_from_sv():
 
     assert m.testing.check_files_equal(__file__, "build/test_pe.sv",
                                        "test_pe.sv")
+
+
+def test_verilog_dependency():
+    [foo, bar] = m.DefineFromVerilog("""
+module foo(input I, output O);
+    assign O = I;
+endmodule
+
+module bar(input I, output O);
+    foo foo_inst(I, O);
+endmodule""")
+    top = m.DefineCircuit("top", "I", m.In(m.Bit), "O", m.Out(m.Bit))
+    bar_inst = bar()
+    m.wire(top.I, bar_inst.I)
+    m.wire(bar_inst.O, top.O)
+    m.EndDefine()
+    FILENAME = "test_verilog_dependency_top"
+    m.compile(f"build/{FILENAME}", top, output="coreir")
+    assert m.testing.check_files_equal(__file__, f"build/{FILENAME}.json",
+                                       f"gold/{FILENAME}.json")