llvm / llvm/circt

[PrefixModules] Prefixing is incorrect

Open
#5,274 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

I understand this is less broken, but please see this through.

Here's a simple tweak to your test that demonstrates one of the kinds of breakages I mentioned: https://github.com/llvm/circt/compare/main...dtzSiFive:circt:misc/prefix-module-breakage?expand=1 .

Running with this PR produces the following (should be caught by verifier (?!)):

```
firrtl.circuit "Prefix_RewriteInnerNameRefs" {
firrtl.module @Prefix_RewriteInnerNameRefs() {
%wire = firrtl.wire sym @wire : !firrtl.uint<1>
firrtl.instance mid @Prefix_Prefix2_Mid()
sv.verbatim "{{0}}" {symbols = [#hw.innerNameRef<@Prefix_RewriteInnerNameRefs::@wire>]}
sv.verbatim "{{0}} {{1}}" {symbols = [#hw.innerNameRef<@Prefix_RewriteInnerNameRefs::@wire>, #hw.innerNameRef<@Prefix_Prefix3_Nested::@wire>]}
}
firrtl.module @Prefix_Prefix2_Mid() {
firrtl.instance nested @Prefix_Prefix2_Prefix3_Nested()
}
firrtl.module @Prefix_Prefix2_Prefix3_Nested() {
%wire = firrtl.wire sym @wire : !firrtl.uint<1>
}
}
```

Notice that in the presence of multiple prefixes being applied the inner ref is updated incorrectly (missing `Prefix2` on `Nested` reference).

_Originally posted by @dtzSiFive in https://github.com/llvm/circt/issues/5259#issuecomment-1563188190_

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the reproducer in the issue and the discussion from issue #5259, then inspect the PrefixModules implementation and its verifier-related tests. Reproduce the nested-prefix case and trace how the inner-name reference is rewritten; done means the shown invalid reference is updated correctly and the verifier catches remaining invalid references.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.