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

Support break in non-parallel for statements by translate range-for into while #578 #583

Merged
merged 24 commits into from Mar 24, 2020

Conversation

archibate
Copy link
Collaborator

Related issue id = #578

@archibate
Copy link
Collaborator Author

Why transform my BinaryOpStmt(cmp_lt) into const [0].

@archibate
Copy link
Collaborator Author

Why transform my BinaryOpStmt(cmp_lt) into const [0].

May because that cond_stmt not inserted into stmts.

@archibate
Copy link
Collaborator Author

Too hard, can't do this at all.

@yuanming-hu
Copy link
Member

Too hard, can't do this at all.

What exactly stopped you? :-)

@archibate
Copy link
Collaborator Author

archibate commented Mar 18, 2020

Too hard, can't do this at all.

What exactly stopped you? :-)

Don't have a clear image of how block (and thus insert_before_me) works.

taichi/transforms/lower_ast.cpp Show resolved Hide resolved
taichi/transforms/lower_ast.cpp Outdated Show resolved Hide resolved
@archibate
Copy link
Collaborator Author

archibate commented Mar 18, 2020

Can I just translate it into FrontendWhileStmt instead of WhileStmt? (in somewhere before lower_ast)

@yuanming-hu
Copy link
Member

Can I just translate it into FrontendWhileStmt instead of WhileStmt? (in somewhere before lower_ast)

Actually, after taking a look at your current implementation, I think it's very very close! We should probably stick to the current one.

@archibate
Copy link
Collaborator Author

Can I just translate it into FrontendWhileStmt instead of WhileStmt? (in somewhere before lower_ast)

Actually, after taking a look at your current implementation, I think it's very very close! We should probably stick to the current one.

Close? Don't understand why it make my BinaryOpStmt becomes const [0].. May be closer if this mystery could reason..

@archibate
Copy link
Collaborator Author

Seems whilestmt used a diff way from forstmt to insert: insert-before-me and flattened. Go bed now.

@yuanming-hu
Copy link
Member

Thanks. I'll take a look after a meeting.

@yuanming-hu
Copy link
Member

Is there a way to reproduce your issue? Which test case should I run? I think your implementation looks promising. Just need to figure out what's wrong in detail.

@yuanming-hu
Copy link
Member

Added a simple test and rebase your commits after code reformatting to avoid future merge conflicts. Please update locally.

@archibate
Copy link
Collaborator Author

archibate commented Mar 19, 2020

You've made TAB=4??! Great to know the process of style enforcing but I would suggest to use TAB="\t" so that user can make themselves happy with editor settings..

@archibate
Copy link
Collaborator Author

Done! Run ninja -C build && ti test for_break -v --arch opengl to reproduce.

@archibate
Copy link
Collaborator Author

Seems whilestmt used a diff way from forstmt to insert: insert-before-me and flattened. Go bed now.

Exactly this caused problem, fixed using flattened2.

@archibate
Copy link
Collaborator Author

Next: how to detect if is_good_range_for.

@archibate
Copy link
Collaborator Author

Can we disable print_ir for snode accessors?

@yuanming-hu
Copy link
Member

Can we disable print_ir for snode accessors?

I think that's already disabled by default?

@archibate
Copy link
Collaborator Author

I think that's already disabled by default?

No, still print something like 2d_ten_arr.

@archibate
Copy link
Collaborator Author

Hello? Have time tonight.

@archibate
Copy link
Collaborator Author

Fixed one, stupid error.

@archibate
Copy link
Collaborator Author

archibate commented Mar 20, 2020

If begin < end, we enumerate [begin, end).
If end > begin, we enumerate [end, begin) or (end, begin]?

@archibate
Copy link
Collaborator Author

Do we have IncStmt for something like x++? Or at least AugAssignStmt for x += 1?

@archibate
Copy link
Collaborator Author

Can we merge this now?

@archibate
Copy link
Collaborator Author

archibate commented Mar 24, 2020

unordered_set?? Wasn't set itself already means unordered? Sorry, I'm not a C++ expert..

@yuanming-hu
Copy link
Member

yuanming-hu commented Mar 24, 2020

unordered_set?? Wasn't set itself already means unordered? Sorry, I'm not a C++ expert..

Well, set is actually ordered: https://en.cppreference.com/w/cpp/container/set std::set is an associative container that contains a sorted set of unique objects of type Key.

Can we merge this now?

Will do it once CI passes. Just now rebased on master to avoid merge conflicts. This will be shipped with v0.5.9 tonight/tomorrow.

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

3 participants