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

[Opt] Make full_simplify iterative when advanced_optimization=True #1225

Merged
merged 2 commits into from
Jun 12, 2020

Conversation

xumingkuan
Copy link
Contributor

@xumingkuan xumingkuan commented Jun 12, 2020

Related issue = N/A

This PR offers shorter compilation time when advanced_optimization=False (by removing a full_simplify pass), and makes full_simplify more "full simplified".

Benchmark: the running time of a large program.

  • advanced_optimization=True:
    • Before: total running time = 6m51s, compilation time of a large kernel = 6m37s
      codegen_accessor_statements: 148.00
      codegen_evaluator_statements: 102.00
      codegen_kernel_statements: 29020.00
      codegen_offloaded_tasks: 53.00
      codegen_statements  : 29270.00
      
    • After: total running time = 6m51s, compilation time of a large kernel = 6m35s
      codegen_accessor_statements: 148.00
      codegen_evaluator_statements: 102.00
      codegen_kernel_statements: 29020.00
      codegen_offloaded_tasks: 53.00
      codegen_statements  : 29270.00
      
  • advanced_optimization=False:
    • Before: total running time = 3m21s, compilation time of a large kernel = 3m7s
      codegen_accessor_statements: 175.00
      codegen_kernel_statements: 59287.00
      codegen_offloaded_tasks: 41.00
      codegen_statements  : 59462.00
      
    • After: total running time = 2m59s, compilation time of a large kernel = 2m46s
      codegen_accessor_statements: 175.00
      codegen_kernel_statements: 59288.00
      codegen_offloaded_tasks: 42.00
      codegen_statements  : 59463.00
      

benchmark20200611

TODO (not urgent): make each IR transform pass return whether it modifies the IR (can be done together with #1059 )

@xumingkuan xumingkuan marked this pull request as draft June 12, 2020 02:02
@xumingkuan xumingkuan marked this pull request as ready for review June 12, 2020 02:04
@xumingkuan
Copy link
Contributor Author

xumingkuan commented Jun 12, 2020

Why didn't CI rerun when a worker crashed? All tests passed on my laptop.

Copy link
Collaborator

@TH3CHARLie TH3CHARLie left a comment

Choose a reason for hiding this comment

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

Looks great! I'll later rebase my local PR for #938 on master once this is merged. And you may push an empty commit to retrigger CI(I encountered the same appveyor issue two days ago).

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.

throw-less treatment in full_simplify LGTM!

@codecov
Copy link

codecov bot commented Jun 12, 2020

Codecov Report

Merging #1225 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1225   +/-   ##
=======================================
  Coverage   67.09%   67.09%           
=======================================
  Files          35       35           
  Lines        4835     4835           
  Branches      886      886           
=======================================
  Hits         3244     3244           
  Misses       1417     1417           
  Partials      174      174           

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 bff5755...9633c18. Read the comment docs.

@xumingkuan xumingkuan merged commit 4d427a2 into taichi-dev:master Jun 12, 2020
@FantasyVR FantasyVR mentioned this pull request Jun 14, 2020
@xumingkuan xumingkuan deleted the simp branch June 14, 2020 22:51
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