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

Enhance Loop Unroller #477

Merged
merged 21 commits into from
Nov 16, 2021
Merged

Enhance Loop Unroller #477

merged 21 commits into from
Nov 16, 2021

Conversation

mostafaelhoushi
Copy link
Contributor

@mostafaelhoushi mostafaelhoushi commented Nov 2, 2021

We have noticed that the unrolling factor passed to opt as --unroll-count=<num> cli option doesn't always take effect.
This PR provides a custom LLVM loop unroller that enforces the unroll count factor provided in an action..

To run custom unroller:

bazel run //examples/example_unrolling_service/loop_unroller:loop_unroller -- <input>.ll -o <output>.ll  --funroll-count=<num>

To run end-to-end example:

bazel run //examples/example_unrolling_service:example

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 2, 2021
@codecov-commenter
Copy link

codecov-commenter commented Nov 2, 2021

Codecov Report

Merging #477 (724f23c) into development (94c9e79) will increase coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #477      +/-   ##
===============================================
+ Coverage        87.75%   87.93%   +0.17%     
===============================================
  Files              111      111              
  Lines             6181     6182       +1     
===============================================
+ Hits              5424     5436      +12     
+ Misses             757      746      -11     
Impacted Files Coverage Δ
compiler_gym/envs/llvm/compute_observation.py 89.58% <0.00%> (+0.22%) ⬆️
compiler_gym/envs/compiler_env.py 89.95% <0.00%> (+0.47%) ⬆️
...loop_tool/service/loop_tool_compilation_session.py 89.93% <0.00%> (+1.34%) ⬆️
compiler_gym/service/connection.py 79.39% <0.00%> (+2.36%) ⬆️

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 94c9e79...724f23c. Read the comment docs.

@mostafaelhoushi
Copy link
Contributor Author

Update: Thanks for @hughleat for his help. Counting the loops is now solved. Next step is to update the loop metadata

Copy link
Contributor

@ChrisCummins ChrisCummins left a comment

Choose a reason for hiding this comment

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

I had a pick through the code and it all looks good to me (minor comments inline). I'm guessing with this being WIP that the glue code to interface with an environment is coming later. Just ping me whenever you want me to take a look through :-)

Cheers,
Chris

@mostafaelhoushi mostafaelhoushi changed the title WIP: Enhance Loop Unroller Enhance Loop Unroller Nov 12, 2021
@mostafaelhoushi mostafaelhoushi marked this pull request as ready for review November 12, 2021 03:23
@mostafaelhoushi
Copy link
Contributor Author

I had a pick through the code and it all looks good to me (minor comments inline). I'm guessing with this being WIP that the glue code to interface with an environment is coming later. Just ping me whenever you want me to take a look through :-)

Cheers, Chris

Thanks Chris. I have addressed all the comments.
Yes, now I have updated the environment to call the custom unroller. I have also made it optional in case someone wants to use LLVM's default unroller

@mostafaelhoushi mostafaelhoushi merged commit 5409415 into development Nov 16, 2021
@mostafaelhoushi
Copy link
Contributor Author

Just ping me whenever you want me to take a look through :-)

I merged it without pinging you :(
Do you want to take another look and make changes?

@ChrisCummins
Copy link
Contributor

I merged it without pinging you :( Do you want to take another look and make changes?

No worries! Code looks great :) I'll try it out and let you know if i encounter any problems

Cheers,
Chris

@ChrisCummins ChrisCummins deleted the unrolling-granularity branch November 16, 2021 16:54
@ChrisCummins ChrisCummins added this to Done (not yet shipped) in CompilerGym roadmap Nov 17, 2021
This was referenced Nov 17, 2021
@ChrisCummins ChrisCummins moved this from Done (not yet shipped) to Shipped in CompilerGym roadmap Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants