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

Compaction Accounts for Continuous Assignments #1847

Merged
merged 8 commits into from
Jan 12, 2024
Merged

Conversation

calebmkim
Copy link
Contributor

@calebmkim calebmkim commented Jan 12, 2024

Fix #1767. Adds analysis so that compaction accounts for continuous assignments.

The analysis here is conservative, since it essentially treats all the continuous assignment as one big "block". In other words, any time a group affects the continuous assignments, then it will depend on any previous groups that affect on continuous assignments too. For example say we have

seq {
  A; // Affects continuous assignments
  B; 
  C; // Affects continuous assignments
  D; 
  E; // Affects continuous assignments
} 

Then we detect an A->C->E dependence, in addition to the existing dependencies we already find by analyzing the groups/control.

When I say a group "affects" a continuous assignment, I mean that the group reads or writes to a cell that continuous assignments read or write from (except in the case when the group and continuous assignments are just reading from the same cell; that's the only time that doesn't count).

Also, I checked and this doesn't affect the polybench results from our paper.

@calebmkim calebmkim changed the title Compaction Accounts for Continuous Assignments (Fix #1767) Compaction Accounts for Continuous Assignments Jan 12, 2024
Copy link
Contributor

@rachitnigam rachitnigam left a comment

Choose a reason for hiding this comment

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

LGTM! It occurs to me that we should sit down at some point and review all the analyses and restructure them a bit to be more reusable across passes. The MO with ReadWriteSet has been adding ad-hoc methods every time we have new need.

@calebmkim calebmkim merged commit c945871 into main Jan 12, 2024
7 checks passed
@calebmkim calebmkim deleted the compaction-bug branch January 12, 2024 21:46
rachitnigam pushed a commit that referenced this pull request Feb 16, 2024
* first pass

* code cleaning

* documentation

* clippy

* bug fix

* better analyssi

* documentation
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.

Compaction does not account for continuous assignments
2 participants