[reduce] Improve handling of NLA's, especially "new" format + sharing.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
HierPathOp's (previously `firrtl.nla`) no longer are referenced via `circt.nonlocal` breadcrumbs along the instance path, so identifying which hierpath's an instance participates in cannot be determined by checking for that. For example, this causes ModuleExternalizer to no longer be able to externalize instances if an NLA targets the ports/ops of the module (or its children, but not sure that would work previously (?)). Similarly InstanceStubber, so on.
In addition to dropping the breadcrumbs, to reduce the quantity of HierPathOp's and cost of their maintenance, they now end in the module now (instead of InnerRef / (Symbol::InnerSymbol pair)), identifying the path only not the targets specifically, so they're often shared across operations/ports using them. If this can be handled reasonably would help avoid invalid IR but may require more expensive walks to be done reliably.
FIRRTL's `NLATable` may be useful for querying the HierPath's an instance participates in, but finding other users may require walking the final module and/or looking through cached SymbolUsers (or so). Or extending NLATable.
This may not be worth addressing if we change how these work, but recording this issue since ran into it trying to use circt-reduce on NLA-heavy designs.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with ModuleExternalizer and InstanceStubber, then inspect FIRRTL's NLATable and the current HierPathOp handling. Reproduce the problem with circt-reduce on NLA-heavy designs and determine how to account for shared, module-terminated paths; done means these operations handle the new format without producing invalid IR.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100