Apply identifier-length to basic block module and container instance names (fix for #122)#124
Conversation
@microsoft-github-policy-service agree company="Microsoft" |
There was a problem hiding this comment.
Pull request overview
Applies the --identifier-length clamping behavior more consistently across generated CIRCT/Verilog identifiers to avoid downstream tool name-length limit failures (e.g., Quartus), specifically for basic-block module naming and container instance symbol names.
Changes:
- Clamp basic-block instance names and basic-block module names during Verilog/CIRCT generation.
- Clamp container instance
InnerSymnames when creatingkanagawa::ContainerInstanceOp. - Add a small CIRCT utility helper (
ClampedSymAttr) and clamp fully-qualified symbol attributes used for container ports/fields.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
compiler/cpp/verilog.cpp |
Clamps basic-block module/instance names and container instance InnerSym names to respect --identifier-length. |
compiler/cpp/circt_util.cpp |
Introduces ClampedSymAttr helper and applies clamping to fully-qualified symbol strings used in CIRCT attrs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The SV spec specifies a limit of 1024 characters it turns out! |
--identifier-length was not being applied to all generated instance names, resulting in generated code that could violate downstream tool (e.g. Quartus) name length limit checks. This PR applies the identifier-length also to basic-block module names and container instance names,