-
Notifications
You must be signed in to change notification settings - Fork 276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FSM]New builders for StateOp and TransitionOp. #6991
base: main
Are you sure you want to change the base?
Conversation
1. Create an OutputOp inside a StateOp with the output values instead of an empty OutputOp, which needs to be erased after all. 2. Accept two functions as arguments to create the TransitionOp, which is similar to the creation of sv::IfOp. This would be helpful because one doesn't have to create the blocks and set the insertion point manually, which may cause errors sometime.
lib/Dialect/FSM/FSMOps.cpp
Outdated
state.addAttribute("nextState", | ||
FlatSymbolRefAttr::get(builder.getStringAttr(nextState))); | ||
} | ||
// void TransitionOp::build(OpBuilder &builder, OperationState &state, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented out code
include/circt/Dialect/FSM/FSMOps.td
Outdated
// OpBuilder<(ins "StringRef":$nextState)>, | ||
OpBuilder<(ins "fsm::StateOp":$nextState)>, | ||
OpBuilder<(ins "StringRef":$nextState, | ||
CArg<"const std::function<void()>&", "{}">:$guardCtor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use llvm::function_ref
instead of const std::function<void()>&
lib/Dialect/FSM/FSMOps.cpp
Outdated
guardCtor(); | ||
} | ||
|
||
Region *actionRegion = state.addRegion(); // guard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// guard
-> // action
?
Thanks @mortbopet, the changes are pushed ;) |
StateOp
, which also takes a ValueRange and uses it to create theOutputOp
with it.TransitionOp
to be able to take twostd::function<void>
as arguments, which are similar to thesv::IfOp
and are by default null. Which means, this change doesn't affect anything already existed.These are simple changes, but would be very helpful when creating these two ops. One doesn't have to always use
ensureGuard
andensureAction
to create the blocks and set the InsertionPoint. Don't mention thatensureGuard
creates an emptyReturnOp
, which in most cases need to be erased every time. The emptyOutputOp
created in StateOp is in most cases useless as well.