llvm / llvm/circt

[ExportVerilog] Symbols defined in `IfDefOp` regions are not added to symbol cache

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

Description

Symbols get added to the symbol cache in `gatherFiles` ([specifically this `TypeSwitch`](https://github.com/llvm/circt/blob/d2e29b53634ca64d66c1d32f57210f4ec40d0fe5/lib/Conversion/ExportVerilog/ExportVerilog.cpp#L4073)). The typeswitch doesn't include logic for `IfDefOp`, which must be recursive. It's not only difficult to make this function recursive it's not clear that it makes sense to let operations not at the top level get emitted to different files.

I think populating the symbol cache should be divorced from gathering the files. (The function documentation for `gatherFiles` doesn't mention populating the symbol cache anyway.) @lattner what do you think?

For reference, I came across this bug in an effort to put `TypeScopeOp` in an `IfDefOp` as a c-style header guard, so really any solution that results in `TypeScopeOp` symbols inside of `IfDefOp`s making it into the symbol cache satisfies my immediate needs. This problem seemed bigger than that.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in lib/Conversion/ExportVerilog/ExportVerilog.cpp at gatherFiles and its TypeSwitch. Trace how symbols are added to the symbol cache, then determine how IfDefOp regions should be handled, including TypeScopeOp symbols. Done means TypeScopeOp symbols inside IfDefOp regions are present in the symbol cache without relying on top-level file gathering.

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.