[FIRRTL] Dedup should properly handle DistinctAttr, including its use in paths.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
In https://github.com/llvm/circt/pull/6582, initial support for classes and objects was added in Dedup.
However, the way we use DistinctAttr with paths is not compatible with Dedup, almost by definition. Two paths, which could dedup if the entities they point to dedup, will have different DistinctAttrs. The initial PR does not handle this, and simply ignores DistinctAttr. We have verifiers and logic in later passes that ensure any paths that came out of dedup are at least valid, but this is not sufficient: Dedup should check, and only dedup two paths if the entities they point to dedup.
I have some thoughts about how to approach this, which I will share here, but for now I want to file this ticket to at least explain the "what", if not the "how".
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reading PR 6582 and the Dedup implementation it introduced, then trace how DistinctAttr is used on paths and how the later verifiers validate them. Done means Dedup checks the entities referenced by paths and only deduplicates paths when those entities can also be deduplicated, rather than ignoring DistinctAttr.
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