systemrdl-compiler icon indicating copy to clipboard operation
systemrdl-compiler copied to clipboard

Signals are not allowed within fields

Open j-klier opened this issue 3 years ago • 1 comments

The 2.0 spec allows signal, enum, and constraints within a field in section 9.1

No other structural component can be defined within a field component; however, signal, enumeration (enum), and constraint components can be defined within a field component

But the FieldComponentVisitor doesn't allow any component definitions. https://github.com/SystemRDL/systemrdl-compiler/blob/main/systemrdl/core/ComponentVisitor.py#L962-L976

I think signals can be allowed with a change similar to this:

class FieldComponentVisitor(ComponentVisitor):
    comp_type = comp.Field

    def visitComponent_insts(self, ctx: SystemRDLParser.Component_instsContext) -> None:
        # Unpack instance def info from parent
        comp_def = self._tmp_comp_def

        # Component instantiations are limited to constraint, and signal instances
        if not isinstance(comp_def, comp.Signal):
            self.msg.fatal(
                "Instantiation of '%s' components not allowed inside a field definition"
                % type(comp_def).__name__.lower(),
                src_ref_from_antlr(ctx.component_inst(0).ID())
            )
        super().visitComponent_insts(ctx)

    def check_comp_def_allowed(self, type_token: 'CommonToken') -> None:
        comp_type = self._CompType_Map[type_token.type]

        # 9.1: No other structural component can be defined within a field component;
        # however, signal, enumeration (enum), and constraint components can be defined within a field component
        if comp_type != comp.Signal:
            self.msg.fatal(
                "Definitions of '%s' components not allowed inside a field definition"
                % comp_type.__name__.lower(),
                src_ref_from_antlr(type_token)
            )

j-klier avatar Jul 07 '22 22:07 j-klier

Thanks for pointing this out! I missed the detail in the spec that you pointed out. Agree this is a bug.

amykyta3 avatar Aug 18 '22 05:08 amykyta3

Fixed in v1.25.0

amykyta3 avatar Oct 18 '22 05:10 amykyta3

Thanks for fixing this. I noticed that the error message should say "field" instead of "reg", correct?

https://github.com/SystemRDL/systemrdl-compiler/commit/4d5f5b89cbf39a20d922e769cebf30799f174ef9#diff-d114e1c5c32e03511ab7a15b47e26f679bac28b72015274e3cea5fed681a7c7aR972

j-klier avatar Oct 19 '22 16:10 j-klier

thanks! fixed.

amykyta3 avatar Oct 20 '22 03:10 amykyta3