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

Use target slot analysis to simplify insert-rotate #530

Merged
merged 1 commit into from Mar 23, 2024

Conversation

j2kun
Copy link
Collaborator

@j2kun j2kun commented Mar 13, 2024

Rebased off #526

This gives us a cleaner implementation of the target slot selection, and has a few benefits:

  • Now programs like hamming_distance and the simple_sum from Achieve the optimum rotations for insert-rotate on a simple_sum example #520 materialize their reductions as a sequence of shift-by-1 rotates, which means we can augment rotate-and-reduce to support replacing that with a logarithmic number of rotations. In particular, this increased the number of rotations of the hamming_distance test case by one, which is temporary.
  • I found a way to incorporate the result of a dataflow analysis into DRR rewrite patterns, which is demonstrated in this PR by first running the dataflow solver, then walking the IR and attaching the output of the analysis as attributes to the relevant ops, and adding a helper NativeCodeCall in DRR to extract the attribute. I found out you can also use this to apply constraints (i.e., apply this pattern only if the op has the relevant attr). This method seems like it would be limited to simple data output by the analysis, not something complex like a struct or piece of a larger data structure.

@AlexanderViand-Intel
Copy link
Collaborator

  • I found a way to incorporate the result of a dataflow analysis into DRR rewrite patterns, which is demonstrated in this PR by first running the dataflow solver, then walking the IR and attaching the output of the analysis as attributes to the relevant ops, and adding a helper NativeCodeCall in DRR to extract the attribute. I found out you can also use this to apply constraints (i.e., apply this pattern only if the op has the relevant attr). This method seems like it would be limited to simple data output by the analysis, not something complex like a struct or piece of a larger data structure.

Ohhh, nice! That's a cool little insight, independent of the main point of the PR/pass. We might want to start collecting these things and other "best practices"/"tips and tricks" somewhere so new contributors can discover them?

@j2kun
Copy link
Collaborator Author

j2kun commented Mar 18, 2024

Rebased and ready for any final review comments.

@j2kun j2kun force-pushed the target-slot-analysis-pass branch from 2cbc854 to 48ca56d Compare March 18, 2024 19:50
Copy link
Collaborator

@AlexanderViand-Intel AlexanderViand-Intel left a comment

Choose a reason for hiding this comment

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

Looks good to me! Sorry for the delay, I thought I had reviewed this on Friday before leaving for Toronto, but apparently forgot 🙈

@asraa asraa added the pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing label Mar 23, 2024
@j2kun j2kun force-pushed the target-slot-analysis-pass branch from 48ca56d to eddb716 Compare March 23, 2024 16:05
@copybara-service copybara-service bot merged commit a119175 into google:main Mar 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants