[FIRRTL] ExtractInstances wrapper module name is not prefixed
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
The optional wrapper modules that `ExtractInstances` can generate (e.g. through the `group` field in `ExtractSeqMemsFileAnnotation`) do not get prefixed by `PrefixModules`. See `test/Dialect/FIRRTL/SFCTests/ExtractSeqMems/Compose.fir` for an example.
Doing the prefixing is not that simple, since ideally we would want prefixing to run before extraction. But extraction creates `sv.verbatim` ops with symbol references to modules which prefixing will invalidate. Alternatively one could go the route of GrandCentral views which also runs after prefixing, but where prefixing updates the annotations relevant for GCT views to contain the prefix. This is challenging for extraction though, since instances to be extracted are not all explicitly annotated, and whether or not a prefix has to be added to the wrapper module also depends on whether the location of the wrapper module is inside the area of effect of a prefix annotation.
Ideally we would make `PrefixInstances` update symbols appropriately (#3104) and make `sv.verbatim` robust under hierarchy modification. That would disentangle the ordering requirements of the passes and we could run extraction before prefixing.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with test/Dialect/FIRRTL/SFCTests/ExtractSeqMems/Compose.fir to reproduce the missing prefix on an ExtractInstances wrapper. Read the PrefixModules and PrefixInstances pass behavior alongside extraction and sv.verbatim handling; done means the wrapper module is prefixed without invalidating its symbol references.
Written by the indexing model from the issue text.
Assessment
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 28/100