(Translated by https://www.hiragana.jp/)
[CIR][ThroughMLIR] Support lowering cir.do to scf.while by wenhu1024 · Pull Request #695 · llvm/clangir · GitHub
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

[CIR][ThroughMLIR] Support lowering cir.do to scf.while #695

Closed
wants to merge 2 commits into from

Conversation

wenhu1024
Copy link

This PR implements a simple do-while statement lowering cir.do to scf.while.

@bcardosolopes
Copy link
Member

@ShivaChen @GaoXiangYa @Kritoooo can you please review this?

Comment on lines 290 to 295
rewriter->cloneRegionBefore(DoOp.getCond(), &scfWhileOp.getAfter().back());
rewriter->eraseBlock(&scfWhileOp.getAfter().back());

rewriter->inlineBlockBefore(&scfWhileOp->getRegion(1).back(),
&scfWhileOp->getRegion(0).back(),
scfWhileOp->getRegion(0).back().end());
Copy link
Contributor

@Kritoooo Kritoooo Jun 21, 2024

Choose a reason for hiding this comment

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

It looks like you placed the condition block at the end of the After region and then moved it to the Before region. Why not complete this in one step by placing the condition block directly at the end of the Before region? Of course, the effect is almost the same.

Copy link
Author

@wenhu1024 wenhu1024 Jun 21, 2024

Choose a reason for hiding this comment

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

Sorry, I tried the way you said but got wrong info before. Maybe I need to learn MLIR API deeply.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I'm not very familiar with the SCF dialect, but it seems that a simple do-while loop should not pose any problems (though nesting other control flow statements might cause issues). Maybe we can see what other reviewers think.

// FIXME: can not support nested do-while

auto scfWhileOp = rewriter->create<mlir::scf::WhileOp>(
DoOp.getLoc(), DoOp->getResultTypes(), adaptor.getOperands());
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Nice! I will try that when free.

@bcardosolopes
Copy link
Member

We just did a rebase against upstream, this PR needs to be updated to account for that, sorry for the churn!

@wenhu1024
Copy link
Author

We just did a rebase against upstream, this PR needs to be updated to account for that, sorry for the churn!

I had modified this pr.

@GaoXiangYa
Copy link
Contributor

This pr should be closed, I have dealt with this problems in #756 @bcardosolopes

@bcardosolopes
Copy link
Member

Thanks for clarifying @GaoXiangYa

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.

None yet

5 participants