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

[ir][refactor] Move all frontend stmts to frontend_ir.h #916

Merged
merged 2 commits into from
May 4, 2020

Conversation

k-ye
Copy link
Member

@k-ye k-ye commented May 3, 2020

This is a follow up of #914 . We move all Frontend*Stmt to frontend_ir.h. (Expressions will be handled separately)

Related issue = #689

[Click here for the format server]

@k-ye k-ye requested review from yuanming-hu and archibate May 3, 2020 03:15
Copy link
Collaborator

@archibate archibate left a comment

Choose a reason for hiding this comment

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

Cool! LGTM in general, except for a few nit.

stmt = stmt_tmp.get();
current_ast_builder().insert(std::move(stmt_tmp));
}
explicit If(const Expr &cond);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If and While are still in ir.h? Wasn't it about frontend?

Copy link
Member Author

@k-ye k-ye May 3, 2020

Choose a reason for hiding this comment

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

Thanks! I think these are legacy stuff -- when Taichi was provided in C++14 before integrated in Python3. @yuanming-hu could you confirm?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, they are for the legacy C++14 frontend and should be removed in a future PR.

@@ -1005,4 +1005,34 @@ bool ContinueStmt::as_return() const {
return false;
}

If::If(const Expr &cond) {
auto stmt_tmp = std::make_unique<FrontendIfStmt>(cond);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here. So it should be in frontend_ir.cpp?

Copy link
Member

@yuanming-hu yuanming-hu left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@k-ye
Copy link
Member Author

k-ye commented May 4, 2020

OK, let's leave these legacy stuff in ir.h|cpp, and worry about them later. However, they don't seem to be easily removable, as the svd implementation still relies on these legacy constructs, e.g.

For(0, num_iters, [&](Expr sweep) {

@k-ye k-ye merged commit ecca629 into taichi-dev:master May 4, 2020
@k-ye k-ye deleted the front branch May 4, 2020 08:14
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.

4 participants