llvm / llvm/circt

[FIRRTL] ExtractInstances wrapper module name is not prefixed

Open
#3,105 0 comments 0 reactions 0 assignees View on GitHub
bug FIRRTL
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.