Skip to content

[HWToBTOR2] Avoid invalid BTOR2 when regs have same name#9916

Merged
TaoBi22 merged 2 commits into
llvm:mainfrom
TaoBi22:btor2-symbol-clash
Mar 12, 2026
Merged

[HWToBTOR2] Avoid invalid BTOR2 when regs have same name#9916
TaoBi22 merged 2 commits into
llvm:mainfrom
TaoBi22:btor2-symbol-clash

Conversation

@TaoBi22

@TaoBi22 TaoBi22 commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Currently we emit invalid BTOR2 if two regs have the same name attribute, as the states get assigned the same symbol in BTOR2 and the parser complains (I ran into this using the LowerSeqShiftReg pass, as lowering two shiftregs will produce compreg.ces with the same name). This just works around that by tracking the symbols we've already created and suffixing names that are already in use

@TaoBi22 TaoBi22 requested review from dobios and fabianschuiki March 12, 2026 16:42
@uenoku

uenoku commented Mar 12, 2026

Copy link
Copy Markdown
Member

Please consider using https://github.com/llvm/circt/blob/main/include/circt/Support/Namespace.h. Namespace ns; ns.newName(reg.getName()) should work.

@TaoBi22

TaoBi22 commented Mar 12, 2026

Copy link
Copy Markdown
Contributor Author

Please consider using https://github.com/llvm/circt/blob/main/include/circt/Support/Namespace.h. Namespace ns; ns.newName(reg.getName()) should work.

Ah thank you! I thought I'd seen something like this but looked in MLIR rather than CIRCT. I'll update it to use this.

@dobios dobios left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, I was looking for this exact Namespace thing for the compreg and shiftreg lowerings, it's fine to handle this here, but generally speaking I would prefer to handle this kind of stuff in upstream passes as much as possible to keep this emission pass as easy to maintain as possible

@TaoBi22

TaoBi22 commented Mar 12, 2026

Copy link
Copy Markdown
Contributor Author

Oh nice, I was looking for this exact Namespace thing for the compreg and shiftreg lowerings, it's fine to handle this here, but generally speaking I would prefer to handle this kind of stuff in upstream passes as much as possible to keep this emission pass as easy to maintain as possible

Agreed it would be nice to not run into upstream but since it's valid IR nonetheless I figured support here was probably worthwhile!

@TaoBi22 TaoBi22 merged commit 949788b into llvm:main Mar 12, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants