Skip to content

[mlir][SCF] Report error when lowering to cf in single block op #65305

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

Closed

Conversation

matthias-springer
Copy link
Member

Report an error when lowering an "scf" op inside of a parent that supports only a single block (e.g., "scf.forall", which does currently not have a "cf" lowering). This indicates a problem in the pass pipeline: scf-to-cf is typically one of the last passes to run and ops that do not support unstructured control flow should have been lowered already.

Before this change, the lowering produced IR that does not verify.

Report an error when lowering an "scf" op inside of a parent that supports only a single block (e.g., "scf.forall", which does currently not have a "cf" lowering). This indicates a problem in the pass pipeline: scf-to-cf is typically one of the last passes to run and ops that do not support unstructured control flow should have been lowered already.

Before this change, the lowering produced IR that does not verify.
Copy link
Member

@ftynse ftynse left a comment

Choose a reason for hiding this comment

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

What if the parent op is going to be converted by a later pattern application?

@matthias-springer
Copy link
Member Author

What if the parent op is going to be converted by a later pattern application?

Dialect conversion traverses the IR pre-order.

But generally speaking, another approach would be to add an scf.forall lowering. That's the main op I was concerned about. (And scf-to-cf lowers all SCF loop ops except for that one at the moment.)

@@ -370,6 +373,9 @@ LogicalResult ForLowering::matchAndRewrite(ForOp forOp,
LogicalResult IfLowering::matchAndRewrite(IfOp ifOp,
PatternRewriter &rewriter) const {
auto loc = ifOp.getLoc();
if (ifOp->getParentOp()->hasTrait<OpTrait::SingleBlock>())
return ifOp->emitError(
"cannot lower op inside parent that expects a single block");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use notifyMatchFailure instead

@matthias-springer
Copy link
Member Author

I'm not happy with solution yet, but don't have time to work at this at the moment. The issue with "scf.forall" has been fixed with #65449. Closing this PR for now.

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