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

Compatibility between UDP `default` property and `UDPDefinition.default_assignment`

Open maltaisn opened this issue 9 months ago • 4 comments

Declaring an UDP like:

property my_udp {
    component = reg;
    type = boolean;
    default = false;
};

And registering the UDP definition:

class MyUDP(UDPDefinition):
    name = "my_udp"
    valid_components = {Reg}
    valid_type = bool
    default_assignment = False

This will fail with error:

error: The property definition for the feature extension 'my_udp' uses a different 'default' definition from what this tool expects.

It seems like the comparison is being made with an AST node for false instead of the value itself.

maltaisn avatar May 08 '24 14:05 maltaisn

Thanks! Yeah this looks like it might be a bug. I see in my testcases i do not cover the case where default_assignment is a boolean. Will look into it!

amykyta3 avatar May 08 '24 15:05 amykyta3

In the meantime, double-check whether you meant to use the get_unassigned_default() method instead. That one is subtly different and in my opinion, way more useful than SystemRDL's weird "default assignment" semantics.

  • "default assignment" = what the property gets assigned if the user specifies it without an assignment operator. In your example, my_udp; would imply my_udp = false;
  • "unassigned default" = what the API will return for the property if the user never assigned or specified the property at all.

More analysis on the interpretation of "default assignments" can be found here: https://systemrdl-compiler.readthedocs.io/en/stable/dev_notes/rdl_spec_errata.html#meaning-of-the-user-defined-property-default-attribute

amykyta3 avatar May 08 '24 15:05 amykyta3

Also it appears that when not specifying the default in the UDP, the default default is systemrdl.rdltypes.NoValue instead of false if I'm interpreting Table 7 in the spec correctly: image

maltaisn avatar May 09 '24 14:05 maltaisn

Table 7's "default" column isn't really applicable to UDPs. Rather, that column is generally applied to built-in RDL properties. Even then, I have found that some properties have further clauses described that contradict with Table 7.

I have found that the specification creates several contradictions when it comes to UDPs. Some of them, they call out explicitly which is good. For example clause 15.1.1-d:

The default attribute can result in some inconsistencies relative to the behavior of built-in properties to the language, especially relating to boolean properties. Built-in booleans in SystemRDL are inherently defaulted to false. With user-defined boolean properties, the default can be specified to be true or false. A default of true creates an inconsistency with respect to SystemRDL property assignments.

Regarding an un-specified UDP default implied default assignment, I have made the interpretation that it results in an un-defined value (aka NoValue) based on the example in 15.2.2-Example1:

property a_map_p { type = string; component = addrmap | regfile; };
...
a_map_p; // The property has been bound to the map but it has not been
         // assigned a value so its value is undefined

Given the text in 15.1.1-d and and the above example, the interpretation that is most consistent in my opinion is that if a UDP does not provide a default in its definition, then any implied value assignments will remain undefined, regardless of type.

This is just one of many things that the SystemRDL spec leaves open to interpretation. In the end, I have to do my best to make a fair and sensible interpretation that is least surprising to the user. UDPs are one part of the spec that are pretty sloppily defined so I get a lot of questions about them. I will add it to the growing list of things I have documented here: https://systemrdl-compiler.readthedocs.io/en/stable/dev_notes/rdl_spec_errata.html

amykyta3 avatar May 09 '24 15:05 amykyta3