llvm / llvm/circt

[FIRRTL] Consider a shared helper for descending aggregate types

Open
#4,412 1 comment 0 reactions 0 assignees View on GitHub
enhancement FIRRTL
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

We sometimes have a need to descend potentially nested aggregate types. This can be accomplished using a loop and some arithmetic on `fieldID`. For example, GCT does it here: https://github.com/llvm/circt/blob/8aa5b70057224b06c37293f9a1605cff7a870888/lib/Dialect/FIRRTL/Transforms/GrandCentralTaps.cpp#L1226-L1252

This is also needed in EmitOMIR: https://github.com/llvm/circt/pull/4407

It would be nice to have a shared utility, and use it anywhere we have this pattern, so the `fieldID` arithmetic can be hidden. Potentially this could be something like:

```
LogicalResult walkNestedTypes(FIRRTLType type, size_t fieldID, llvm::function_ref) {
/// while loop and typeswitch from above, call the callback in the branches of the type switch with the current type and index
}
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by comparing the aggregate-type traversal and fieldID arithmetic in lib/Dialect/FIRRTL/Transforms/GrandCentralTaps.cpp around lines 1226-1252 with the corresponding traversal in EmitOMIR from pull request 4407. Identify the shared utility location and verify that both call sites can use it while preserving their existing traversal behavior.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.