address/field collisison validation

.coveragerc

@@ -13,4 +13,9 @@ omit =
     ./examples/*
 
     # Omit Antlr-generated parser templates
-    ./systemrdl/parser/*
+    ./systemrdl/parser/*
+
+[report]
+exclude_lines =
+    raise RuntimeError
+    raise NotImplementedError

systemrdl/compiler.py

@@ -191,7 +191,7 @@ def elaborate(self, top_def_name=None, inst_name=None, parameters=None):
         )
 
         # Validate design
-        walker.RDLWalker().walk(root_node, ValidateListener(self))
+        walker.RDLWalker(skip_not_present=False).walk(root_node, ValidateListener(self))
 
         if self.msg.error_count:
             self.msg.fatal("Elaborate aborted due to previous errors")

systemrdl/component.py

@@ -19,7 +19,14 @@ def __init__(self):
         #: instance's parameter values.
         self.type_name = None
 
-        # Child elements instantiated inside this component
+        #: Child elements instantiated inside this component
+        #: 
+        #: Child components are sorted as follows:
+        #: 
+        #: - Signals first
+        #: - All other components follow.
+        #: - AddressableComponents are sorted by ascending base_addr
+        #: - Fields are sorted by ascending low bit
         self.children = []
 
         # Parameters of this component definition.

systemrdl/core/ComponentVisitor.py

@@ -82,7 +82,7 @@ def visitComponent_def(self, ctx:SystemRDLParser.Component_defContext):
         elif ctx.component_named_def() is not None:
             comp_def = self.visit(ctx.component_named_def())
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
         if ctx.component_insts() is not None:
             # Component is instantiated one or more times
@@ -149,7 +149,7 @@ def define_component(self, body, type_token, def_name, param_defs):
             if subclass.comp_type == self._CompType_Map[type_token.type]:
                 visitor = subclass(self.compiler, def_name, param_defs)
                 return visitor.visit(body)
-        raise RuntimeError # pragma: no cover
+        raise RuntimeError
 
     #---------------------------------------------------------------------------
     # Component Instantiation
@@ -403,7 +403,7 @@ def visitComponent_inst(self, ctx:SystemRDLParser.Component_instContext):
                 comp_inst.array_dimensions = array_suffixes
                 comp_inst.array_stride = inst_addr_stride
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
         if inst_type == SystemRDLParser.EXTERNAL_kw:
             comp_inst.external = True
@@ -415,7 +415,7 @@ def visitComponent_inst(self, ctx:SystemRDLParser.Component_instContext):
                 comp_inst.is_alias = True
                 comp_inst.alias_primary_inst = alias_primary_inst
             else:
-                raise RuntimeError # pragma: no cover
+                raise RuntimeError
 
         self.component.children.append(comp_inst)
 
@@ -533,7 +533,7 @@ def visitLocal_property_assignment(self, ctx:SystemRDLParser.Local_property_assi
                 rhs = rdltypes.InterruptType[prop_mod]
 
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
         if default:
             self.compiler.namespace.register_default_property(prop_name, (prop_token, rhs), prop_token)
@@ -559,7 +559,7 @@ def visitDynamic_property_assignment(self, ctx:SystemRDLParser.Dynamic_property_
         elif ctx.encode_prop_assign() is not None:
             prop_token, prop_name, rhs = self.visit(ctx.encode_prop_assign())
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
         # Lookup component instance being assigned
         target_inst = self.component

systemrdl/core/ExprVisitor.py

@@ -243,7 +243,7 @@ def visitCastType(self, ctx:SystemRDLParser.CastTypeContext):
         elif ctx.typ.type == SystemRDLParser.BOOLEAN_kw:
             return e.BoolCast(self.compiler, ctx.op, self.visit(ctx.expr()))
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
     # Visit a parse tree produced by SystemRDLParser#CastWidth.
     def visitCastWidth(self, ctx:SystemRDLParser.CastWidthContext):
@@ -294,7 +294,7 @@ def visitInstance_ref(self, ctx:SystemRDLParser.Instance_refContext):
                 ref_elements=ref_elements
             )
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
         return ref_expr
 

systemrdl/core/expressions.py

@@ -33,15 +33,15 @@ def predict_type(self):
         even if the child type is not relevant to the resulting type.
         Raises exception if input types are not compatible.
         """
-        raise NotImplementedError # pragma: no cover
+        raise NotImplementedError
 
     def get_min_eval_width(self):
         """
         Returns the expressions resulting integer width based on the
         self-determined expression bit-width rules
         (SystemVerilog LRM: IEEE Std 1800-2012, Table 11-21)
         """
-        raise RuntimeError # pragma: no cover
+        raise RuntimeError
 
     def get_value(self, eval_width=None):
         """
@@ -61,7 +61,7 @@ def get_value(self, eval_width=None):
         - If eval_width is set to a value:
             Parent expression is propagating the eval_width
         """
-        raise NotImplementedError # pragma: no cover
+        raise NotImplementedError
 
 #-------------------------------------------------------------------------------
 class IntLiteral(Expr):
@@ -194,7 +194,7 @@ def get_min_eval_width(self):
 
             return width
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
     def get_value(self, eval_width=None):
         if self.type == int:
@@ -212,7 +212,7 @@ def get_value(self, eval_width=None):
             return result
 
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
 #-------------------------------------------------------------------------------
 class Replicate(Expr):
@@ -242,7 +242,7 @@ def predict_type(self):
         else:
             # All replications contain a nested concatenation
             # Type check for invalid type is already halded there
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
     def get_min_eval_width(self):
         # Evaluate number of repetitions
@@ -255,7 +255,7 @@ def get_min_eval_width(self):
             width *= self.reps_value
             return width
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
     def get_value(self, eval_width=None):
         # Evaluate number of repetitions
@@ -279,7 +279,7 @@ def get_value(self, eval_width=None):
             return result
 
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
 #-------------------------------------------------------------------------------
 # Integer binary operators:
@@ -488,7 +488,7 @@ def get_ops(self):
             l = self.l.get_value()
             r = self.r.get_value()
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
         return l,r
 
@@ -777,7 +777,7 @@ def get_value(self, eval_width=None):
             j = self.j.get_value()
             k = self.k.get_value()
         else:
-            raise RuntimeError # pragma: no cover
+            raise RuntimeError
 
         if i:
             return j

systemrdl/core/namespace.py

@@ -54,7 +54,7 @@ def lookup_element(self, name:str):
                     return (el, parent_def)
                 else:
                     return (None, None)
-        return None
+        return (None, None)
 
     def get_default_properties(self, comp_type):
         """

systemrdl/core/validate.py

@@ -1,6 +1,7 @@
 
 from .. import walker
 from .. import rdltypes
+from ..node import RegNode
 
 #===============================================================================
 # Validation Listeners
@@ -12,27 +13,81 @@ def __init__(self, compiler):
         self.msg = compiler.msg
 
         # Used in field overlap checks
-        self.prev_field = None
+        # This is a rolling buffer of previous fields that still have a chance
+        # to possibly collide with a future field
+        self.field_check_buffer = []
 
+        # Used in addrmap, regfile, and reg overlap checks
+        # Same concept as the field check buffer, but is also a stack
+        self.addr_check_buffer_stack = [[]]
+
+
     def enter_Component(self, node):
 
         # Validate all properties that were applied to the component
         for prop_name in node.inst.properties.keys():
             prop_value = node.get_property(prop_name)
             prop_rule = self.compiler.property_rules.lookup_property(prop_name)
             prop_rule.validate(node, prop_value)
+
+
+    def enter_AddressableComponent(self, node):
+        addr_check_buffer = self.addr_check_buffer_stack[-1]
+        self.addr_check_buffer_stack.append([])
 
-    def enter_Reg(self, node):
-        self.prev_field = None
+        # Check for collision with previous addressable sibling
+        new_addr_check_buffer = []
+        for prev_addressable in addr_check_buffer:
+            if (prev_addressable.inst.addr_offset + prev_addressable.total_size) > node.inst.addr_offset:
+                # Overlaps!
+
+                # Only allowable overlaps are as follows:
+                #   10.1-h: Registers shall not overlap, unless one contains only
+                #   read-only fields and the other contains only write-only or
+                #   write-once-only fields.
+                overlap_allowed = False
+                if isinstance(prev_addressable, RegNode) and isinstance(node, RegNode):
+                    if ((not prev_addressable.has_sw_writable) and (not node.has_sw_readable)
+                        or (not prev_addressable.has_sw_readable) and (not node.has_sw_writable)
+                    ):
+                        overlap_allowed = True
+
+                if not overlap_allowed:
+                    self.msg.error(
+                        "Instance '%s' at offset +0x%X:0x%X overlaps with '%s' at offset +0x%X:0x%X"
+                        % (
+                            node.inst.inst_name, node.inst.addr_offset, node.inst.addr_offset + node.total_size - 1,
+                            prev_addressable.inst.inst_name, prev_addressable.inst.addr_offset, prev_addressable.inst.addr_offset + prev_addressable.total_size - 1,
+                        ),
+                        node.inst.inst_err_ctx
+                    )
+
+                # Keep it in the list since it could collide again
+                new_addr_check_buffer.append(prev_addressable)
+        self.addr_check_buffer_stack[-2] = new_addr_check_buffer
 
+
+    def exit_AddressableComponent(self, node):
+        self.addr_check_buffer_stack.pop()
+        self.addr_check_buffer_stack[-1].append(node)
+
+
+    def enter_Reg(self, node):
+        self.field_check_buffer = []
+
+
     def exit_Reg(self, node):
         # 10.1-c: At least one field shall be instantiated within a register
-        if self.prev_field is None:
+        #
+        # At the end of field overlap checking, at least one entry is guaranteed to
+        # be left over in the field_check_buffer
+        if not self.field_check_buffer:
             self.msg.error(
                 "Register '%s' does not contain any fields" % node.inst.inst_name,
                 node.inst.inst_err_ctx
             )
-
+
+
     def enter_Field(self, node):
         this_f_hw = node.get_property('hw')
         this_f_sw = node.get_property('sw')
@@ -80,23 +135,35 @@ def enter_Field(self, node):
         # 10.1-d: Two field instances shall not occupy overlapping bit positions
         # within a register unless one field is read-only and the other field
         # is write-only.
-        if (self.prev_field is not None) and (self.prev_field.inst.high >= node.inst.low):
-            prev_f_sw = self.prev_field.get_property('sw')
-
-            if((prev_f_sw == rdltypes.AccessType.r)
-                and ((this_f_sw == rdltypes.AccessType.w) or this_f_sw == rdltypes.AccessType.w1)
-            ):
-                pass
-            elif((this_f_sw == rdltypes.AccessType.r)
-                and ((prev_f_sw == rdltypes.AccessType.w) or prev_f_sw == rdltypes.AccessType.w1)
-            ):
-                pass
-            else:
-                self.msg.error(
-                    "Field '%s' overlaps with field '%s'"
-                    % (node.inst.inst_name, self.prev_field.inst.inst_name),
-                    node.inst.inst_err_ctx
-                )
+        #
+        # Scan through a copied list of the field_check_buffer for collisions
+        # If an entry no longer collides with the current node, it can be removed
+        # from the list since fields are sorted.
+        new_field_check_buffer = []
+        for prev_field in self.field_check_buffer:
+            if prev_field.inst.high >= node.inst.low:
+                # Found overlap!
+                # Check if the overlap is allowed
+                prev_f_sw = prev_field.get_property('sw')
+
+                if((prev_f_sw == rdltypes.AccessType.r)
+                    and ((this_f_sw == rdltypes.AccessType.w) or this_f_sw == rdltypes.AccessType.w1)
+                ):
+                    pass
+                elif((this_f_sw == rdltypes.AccessType.r)
+                    and ((prev_f_sw == rdltypes.AccessType.w) or prev_f_sw == rdltypes.AccessType.w1)
+                ):
+                    pass
+                else:
+                    self.msg.error(
+                        "Field '%s[%d:%d]' overlaps with field '%s[%d:%d]'"
+                        % (node.inst.inst_name, node.inst.msb, node.inst.lsb,
+                            prev_field.inst.inst_name, prev_field.inst.msb, prev_field.inst.lsb),
+                        node.inst.inst_err_ctx
+                    )
+                # Keep it in the list since it could collide again
+                new_field_check_buffer.append(prev_field)
+        self.new_field_check_buffer = new_field_check_buffer
 
 
         # 10.1-e: Field instances shall not occupy a bit position exceeding the
@@ -110,4 +177,4 @@ def enter_Field(self, node):
 
 
     def exit_Field(self, node):
-        self.prev_field = node
+        self.field_check_buffer.append(node)