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

[async] More SFG optimizations #1877

Merged
merged 35 commits into from
Sep 19, 2020
Merged

Conversation

yuanming-hu
Copy link
Member

@yuanming-hu yuanming-hu commented Sep 16, 2020

Related issue = #742

List of changes:

  • Added misc/async_mgpcg.py, which is a fully asynchronous MGPCG. Note that the old MGPCG has Python-scope operations such as beta = sum[None], which force the system to synchronize
  • Activating global pointers now changes all the mask states from the SNode to the root
  • StateFlowGraph::optimize_listgen is re-written. Now the ClearListStmt gets removed together with the listgen task
  • StateFlowGraph::optimize_dead_store no longer removes ClearListStmt
  • AsyncEngine::synchronize() no longer do optimize_listgen after fusion, since ListGen with fused serials of ClearList is not yet supported (TODO: support...)
  • Every TaskLaunchRecord now has a unique id. This will be used when generating the dot files. Having a consistent id simplified debugging. StateFlowGraph::Node::launch_id is removed and we display the TaskLaunchRecord id only
  • Program now has a snodes mapping that maps SNode id to SNode pointer
  • StateFlowGraph::extract now takes a bool sort. Note that sorting an intermediate graph (which doesn't have order independency) can lead to wrong results
  • Added StateFlowGraph::demote_activation
  • Added a ConstExprPropagation for demote_activation
  • StateFlowGraph::dump_dot can now visual a state flow chain of a single state only. (TODO: make a standard API? it's harder coded for now. What should the API look like?)

[Click here for the format server]


@yuanming-hu yuanming-hu marked this pull request as draft September 16, 2020 15:22
@codecov
Copy link

codecov bot commented Sep 16, 2020

Codecov Report

Merging #1877 into master will increase coverage by 0.95%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1877      +/-   ##
==========================================
+ Coverage   43.25%   44.21%   +0.95%     
==========================================
  Files          44       44              
  Lines        6320     6111     -209     
  Branches     1092     1092              
==========================================
- Hits         2734     2702      -32     
+ Misses       3416     3240     -176     
+ Partials      170      169       -1     
Impacted Files Coverage Δ
python/taichi/lang/ast_checker.py 70.58% <0.00%> (-1.64%) ⬇️
python/taichi/testing.py 75.00% <0.00%> (-0.72%) ⬇️
python/taichi/lang/linalg.py 89.33% <0.00%> (-0.67%) ⬇️
python/taichi/lang/meta.py 62.31% <0.00%> (-0.54%) ⬇️
python/taichi/lang/__init__.py 41.94% <0.00%> (-0.51%) ⬇️
python/taichi/misc/util.py 17.48% <0.00%> (-0.26%) ⬇️
python/taichi/misc/task.py 0.00% <0.00%> (ø)
python/taichi/lang/shell.py 0.00% <0.00%> (ø)
python/taichi/tools/patterns.py 0.00% <0.00%> (ø)
python/taichi/lang/kernel.py 71.17% <0.00%> (+0.13%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17c9fb7...9e5cfb9. Read the comment docs.

@yuanming-hu yuanming-hu marked this pull request as ready for review September 18, 2020 00:30
Copy link
Contributor

@xumingkuan xumingkuan left a comment

Choose a reason for hiding this comment

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

Great!! LGTMig.

misc/visualize_state_flow_graph.py Show resolved Hide resolved
taichi/program/state_flow_graph.cpp Outdated Show resolved Hide resolved
Copy link
Member

@k-ye k-ye left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, I'm still catching up this part, so please feel free to merge! I left a question on the activation demotion..

continue;

auto list_node = *node->input_edges[list_state].begin();
tasks[std::make_pair(node->rec.ir_handle, list_node)].push_back(node);
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm, if there's such a launch sequence:

@ti.kernel
def foo():
  for i in x:
    y[i] = ...

@ti.kernel
def bar():
  for i in range(...):
    if some_cond(i):
      x[i] = ...  # potentially changes x's mask/list state

foo()
bar()
foo()  # <-- cannot demote the activation of |y| now

Does this not demote activation as expected?

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe the things will go as expected (although not tested). The second foo() uses a different list of x, and activation demotion will only happen when the same list states are used :-)

(Btw we should set up a testing system so that cases like this can be tested...)

taichi/program/state_flow_graph.cpp Outdated Show resolved Hide resolved
@yuanming-hu yuanming-hu merged commit 1b78447 into taichi-dev:master Sep 19, 2020
@yuanming-hu yuanming-hu mentioned this pull request Sep 19, 2020
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