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

Schedule compaction #1722

Merged
merged 20 commits into from
Oct 12, 2023
Merged

Schedule compaction #1722

merged 20 commits into from
Oct 12, 2023

Conversation

paili0628
Copy link
Contributor

@paili0628 paili0628 commented Sep 11, 2023

implements #1717.
I have not yet put the schedule-compaction pass into the default pipeline but I think it should be immediately after static-promotion, since I think it is only reasonable to compact static seqs that were promoted by the static-promotion pass, but I want to know what you think @calebmkim

Update: Already inserted schedule-compaction into default pipeline after static-promotion.

On Monday we discussed that while the current version of schedule-compaction is adequate, it is worthwhile to extend this optimization to nested static seqs with repeat, par, seq and while blocks as children. To address the problem that we can only utilize the go signal of groups, @calebmkim suggested that instead of making a big group scheduling go signals, we use a static par cleverly to achieve the same effect.

@rachitnigam
Copy link
Contributor

@paili0628 any updates on the state of this and what would it take to merge this? I think it would be good to summarize the discussion we had in the meeting here to describe the 3 steps it would take to get the complete schedule compaction we wanted.

@paili0628
Copy link
Contributor Author

I'm currently debugging this pr. It seems that for some test cases the while loops are violating the bound constraints even though schedule-compaction does not seem to have changed the code 'visibly'.

@calebmkim
Copy link
Contributor

Is it possible that it's not keeping track of dependencies across iterations of the while loop? For example:

while port {
    A; // reads from register. This read from A depends on the write to the register in C. 
    B;
    C; // writes to register. 
} 

I haven't looked at the code carefully; maybe you have covered this.

@paili0628
Copy link
Contributor Author

paili0628 commented Sep 17, 2023

@calebmkim The problem is that while loops with comb groups are replaced before schedule-compaction, leading to schedule-compaction messing up the scheduling of cond in some cases. The main problem is that some groups write to other groups' go signals, thereby triggering the group, so theoretically the triggered group's read/write cells should be included in the group's read/write cells. For example:

group A {
 a_reg.in = 1'd1;
 a_reg.write_en = 1'd1;
 A[done] = a_reg.done;
}

group B {
 A[go] = 1'd1;
 B[done] = A[done];
}

For the program segment above, group B's write cells should include a_reg when we are doing data dependency analyses, but in the function ReadWriteSet::port_write_set, a_reg is clearly not considered as a write cell. This is making some of the programs fail because our analysis considers B to have no data dependency on any group that is using a_reg. So there are 2 solutions:

  1. make ReadWriteSet::port_write_set recursively find all cells that a group should 'indirectly' write to when seeing a hole.
  2. give up analysis if we see a hole

@calebmkim
Copy link
Contributor

I see. I think we should probably try to handle this case? (Although I'm not sure how difficult that would be).

I had a similar problem when I was trying to use graph coloring to reuse FSMs for static islands that we know wouldn't run at the same time. (In other words, when instantiating FSMs for static islands, I made it so that different static islands can use the same FSM as long as they're guaranteed not be active at the same time). The code is here. Looking back, the code is not necessarily the best, so I could envision factoring out some of this code into a separate analysis that this pass will use as well.

@paili0628
Copy link
Contributor Author

I see what you're saying. I think the code in compile-static is definitely helpful. The only problem is that, I think the current ReadWriteSet::port_write_set function only takes in an Iterator of assignments. In order to obtain all the cells in its 'referred' groups, it will have to take in the component as well, which might affect how other passes or analyses use this function. This is just a potential difficulty and does not mean it's not doable.

@rachitnigam
Copy link
Contributor

@paili0628 are there technical discussions we need to have to move this PR along? Let's try to wrap this up soon so we can start working on results

@paili0628
Copy link
Contributor Author

There are no technical discussions. I only have to deal with fmt. Sorry about this.

@rachitnigam
Copy link
Contributor

No worries!

@paili0628
Copy link
Contributor Author

This is ready for performance evaluation. @calebmkim

@calebmkim
Copy link
Contributor

Awesome thanks!

@paili0628 paili0628 merged commit fb9add4 into master Oct 12, 2023
@paili0628 paili0628 deleted the schedule-compaction branch October 12, 2023 18:32
rachitnigam pushed a commit that referenced this pull request Feb 16, 2024
* added schedule compaction pass

* debug

* fmt

* changed test cases and added schedule-compaction to default pipeline

* addressed comments

* fmt

* debug

* debug

* changed the group enable to static par

* allowed non-enable control and recalculated latency

* add schedule-compaction to default pass

* restored invoke-memory.futil

* fixed bug

* fmt

* restore pass ordering

* debug

* fmt and fix test cases

* fix test cases

* fix test cases
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.

Schedule Compaction Using Static Control
3 participants