Skip to content

Conversation

AtticusKuhn
Copy link
Contributor

This PR adds a pass to circt-opt called --convert-core-to-fsm. This pass converts code written in RTL (comb + seq) and converts it to the fsm dialect.

It recognises and extracts FSM structure from an RTL netlist. It has similar functionality to fsm_extract in yosys:
https://yosyshq.readthedocs.io/projects/yosys/en/0.46/cmd/fsm.html

@AtticusKuhn AtticusKuhn requested a review from darthscsi as a code owner August 14, 2025 14:07
@AtticusKuhn AtticusKuhn marked this pull request as draft August 14, 2025 14:07
Copy link
Contributor

@TaoBi22 TaoBi22 left a comment

Choose a reason for hiding this comment

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

Thanks for making the PR! I haven't properly reviewed the pass code yet (I'll try to do that soon) but in the meantime I have some high-level comments

Copy link
Contributor

@TaoBi22 TaoBi22 left a comment

Choose a reason for hiding this comment

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

(Still haven't done a detailed pass, just some things I noticed)

@TaoBi22 TaoBi22 changed the title [Feat][circt-opt] Add pass --convert-core-to-fsm [circt-opt] Add pass --convert-core-to-fsm Aug 14, 2025
Comment on lines +387 to +399
moduleOp.walk([&](circt::seq::CompRegOp reg) {
if (reg.getName()->contains("state")) {
stateRegs.push_back(reg);
} else {
variableRegs.push_back(reg);
}
});
if (stateRegs.empty()) {
emitError(moduleOp.getLoc())
<< "Cannot find state register in this FSM. You might need to "
"manually specify which registers are state registers.\n";
return mlir::failure();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This analysis does not look reliable. Is it possible to replace it with an analogue of fsm_detect from yosys?

Copy link
Contributor Author

@AtticusKuhn AtticusKuhn Aug 14, 2025

Choose a reason for hiding this comment

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

Yes, that's true, but I think that to have modularity, it would be fine to split up the total pipeline into fsm_detect and fsm_extract (like Yosys does). So, theoretically there could be a pass circt-opt --annotate-fsm-state-registers that works in combination with circt-opt --convert-core-to-fsm. In the meantime, it's not too difficult for the user to manually annotate the FSM state registers in an RTL design.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe for now we could just require it to always have the name of the state register explicitly given by the user? That way we don't need to worry about users having it opaquely pick up a register they don't expect, and we can always later change it to a flag that will get it to look for an attribute

@AtticusKuhn AtticusKuhn marked this pull request as ready for review August 15, 2025 15:39
Atticus Kuhn added 2 commits August 27, 2025 16:16
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.

5 participants