Skip to content
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

[SCFToCalyx] Lower scf.if op when it yields nothing #8079

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

jiahanxie353
Copy link
Contributor

This patch lowers scf.ifOp when it has an empty yield/does not return any value.

@jiahanxie353 jiahanxie353 changed the title [SCFToCalyx] Lower scf.if op it yields nothing [SCFToCalyx] Lower scf.if op when it yields nothing Jan 14, 2025
@jiahanxie353 jiahanxie353 self-assigned this Jan 14, 2025
@jiahanxie353 jiahanxie353 added the Calyx The Calyx dialect label Jan 14, 2025
Copy link
Member

@cgyurgyik cgyurgyik left a comment

Choose a reason for hiding this comment

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

There's a lot of control flow going on in this method making it difficult to follow. I think it can be simplified / split apart in smaller functions, but that wasn't your doing so feel free to leave it. LGTM.

Comment on lines +1171 to +1172
return yieldOp.getOperation()->emitError()
<< "Unsupported empty yieldOp outside ForOp or IfOp.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return yieldOp.getOperation()->emitError()
<< "Unsupported empty yieldOp outside ForOp or IfOp.";
return "Unsupported empty yieldOp: "
<< yieldOp.getOperation()->emitError();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left this unchanged because I think we can only put << after emitError not infront

lib/Conversion/SCFToCalyx/SCFToCalyx.cpp Outdated Show resolved Hide resolved
@jiahanxie353 jiahanxie353 merged commit 09790a7 into llvm:main Jan 15, 2025
4 checks passed
@jiahanxie353 jiahanxie353 deleted the empty-yield-if branch January 15, 2025 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Calyx The Calyx dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants