-
Notifications
You must be signed in to change notification settings - Fork 311
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
Add passes to strip OM and Emit dialect ops #8121
Conversation
namespace { | ||
struct StripEmitPass : public emit::impl::StripEmitPassBase<StripEmitPass> { | ||
void runOnOperation() override { | ||
for (auto &op : llvm::make_early_inc_range(getOperation().getOps())) |
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.
This strips at one level, would this make sense to be a walk
instead?
Also, no need to remove attributes (since there aren't any, I think) is that right?
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.
Yeah good point. I chose to go with a scan over the top-level since we don't (or even can't according to the HasParent traits) have Emit/OM ops nested within modules, to avoid having to do a full walk of the IR. We could switch this to a walk later if needed, with appropriate skips over modules and other things we know can never contain Emit/OM ops.
I haven't found any attributes that need removing yet 🤔
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.
A symbol on emit.fragment
is captured in emit.fragments
attribute in HWModuleOp though it doesn't cause a verification error as it's not proper SymbolUse. I ideally we should delete them but I don't know the best way to deal with this kind of unspecified use of symbols.
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.
Oh nice, thanks for pointing that out. If it's a dialect attribute like emit.fragments
, we could just generally delete them. Or to be more conservative, we could specifically walk HWModuleOp
s and remove that specific op. WDYT?
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.
I think deleting dialect attributes attached as discardable sounds good.
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.
Done! I specifically only delete emit.fragments
on HWModuleOp right now to not walk the entire IR for now. Do you think that'll be enough?
f0901fa
to
888e3aa
Compare
Several tools in CIRCT do not process the OM and Emit dialects at all, for example, arcilator, circt-bmc, and circt-lec. Since these tools do a full lowering to LLVM internally, they have to discard all OM and Emit ops at some point in the pipeline. Arcilator currently does this in its StripSV pass; circt-bmc and circt-lec simply error out. This commit adds a `StripOM` pass to the OM dialect and a `StripEmit` pass to the Emit dialect which remove any OM and Emit ops from the IR, respectively. These passes are added to the arcilator, circt-bmc, and circt-lec pipelines in order to discard OM and Emit from the input. The commit also includes a few boilerplate changes and alphabetical ordering of dialects and dependencies.
888e3aa
to
b9bb9fb
Compare
Several tools in CIRCT do not process the OM and Emit dialects at all, for example, arcilator, circt-bmc, and circt-lec. Since these tools do a full lowering to LLVM internally, they have to discard all OM and Emit ops at some point in the pipeline. Arcilator currently does this in its StripSV pass; circt-bmc and circt-lec simply error out.
This commit adds a
StripOM
pass to the OM dialect and aStripEmit
pass to the Emit dialect which remove any OM and Emit ops from the IR, respectively. These passes are added to the arcilator, circt-bmc, and circt-lec pipelines in order to discard OM and Emit from the input.The commit also includes a few boilerplate changes and alphabetical ordering of dialects and dependencies.
@uenoku Maybe this is useful for
circt-synth
as well?