chipsalliance / synlig

SystemVerilog synthesis tool
Apache License 2.0
169 stars 21 forks source link

Feature Request: Generalize concept of xxxx_select/actual_group relation #1743

Closed hs-apotell closed 1 year ago

hs-apotell commented 1 year ago

Feature Request: Generalize concept of xxxx_select/actual_group relation

Issue

Surelog currently implements this model hierarchy for referenced objects -

ref_obj holds the referenced object as ref_obj::Actual_group() bit_select holds the referenced object as bit_select::VpiParent()::Actual_group() part_select and indexed_part_select follow the same pattern as bit_select

vpiParent relation is considered weak reference and is explicitly ignored in all implemented traversal algorithms (including VpiListener, UhdmListener and vpi_visitor). In current implementation, actual object reference is not printed as part of the UHDM output.

Special handling is required in all client applications for select types, thus opening doors to runtime bugs. To access expected bit_select parent, use bit_select::VpiParent()::VpiParent(), instead of the usual object::VpiParent() To access expected bit_select actual object reference, use bit_select::VpiParent()::Actual_group(), instead of the usual object::Actual_group() like as in ref_obj

Proposed Solution

Generalize the referenced object concept by changing the class hierarchy to the following -

In this solution, ref_obj and its sub classes are all concrete implementations. vpiParent relation continues as a weak reference, needing no special handling. This solution also allows for simplifying the binding logic for all select models. Current binding logic only works for ref_obj and does not account for other select types.

Change in Plugin implementation

To retrieve bit_select parent, instead of bit_select::VpiParent()::VpiParent() use bit_select::VpiParent() Similar edits for part_select and indexed_part_select.

Open UHDM and Surelog PRs

https://github.com/chipsalliance/UHDM/pull/949 https://github.com/chipsalliance/Surelog/pull/3670

kbieganski commented 1 year ago

The linked Surelog and UHDM PRs are merged, and we have an up-to-date Surelog passing in CI, so I assume this is done. If not, please open a new issue.