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

Done conditions are distinguished #1326

Closed
wants to merge 22 commits into from
Closed

Done conditions are distinguished #1326

wants to merge 22 commits into from

Conversation

rachitnigam
Copy link
Contributor

@rachitnigam rachitnigam commented Dec 28, 2022

Done conditions are special and not quite like assignments. This PR updates the ir::Group structure to make done conditions a field so that we don't have to do the janky "iterate over the group assignments to find the done condition" nonsense.

Also, more importantly, we no longer allow a group's done condition to have guards. The textual programs can still write something like:

group upd {
  upd[done] = c.done & d.done ? 1
}

But the compiler immediately canonicalizes it to:

cells {
  dwc = wire(1)
}
group upd {
  upd[done] = dwc.out
}
dwc.in = c.done & d.done ? 1;

There are a couple of benefits of this change. First, we don't need the group[done] hole anymore; we can simply inline the group's done condition directly into the location since the condition is guaranteed to be a port. It also makes sure that figuring out if a group is done straightforward since it does not require any computation.

Finally, w.r.t. undefined behavior, it clarifies the responsibility of the port–it must always have a non-x value. The guard does not play a role in defining the "doneness" of a group.

Fixes #1119 and fixes #864.

TODO

  • Fix rewriting of ports
  • Update tests to have done conditions
  • ReadWriteSet::uses on group assignments returns different results

@rachitnigam
Copy link
Contributor Author

I'm not sure if this incarnation of solving #864 is a good idea. A couple of problems:

  1. Iterating over the group's assignments was a guaranteed way of getting all the used ports. By making done_cond a separate field, we lose this uniformity.
  2. The above canonicalization, where guarded assignments to the group's done condition are externalized to the group, means that we'll have to assume extra-long live ranges for certain cells even though they are only used in a specific group disabling a bunch of sharing possibilities.

I think solving #864, or even attempting to solve it, requires a more thoughtful plan than this PR. I'll try to extract the useful changes from this PR to solve #1119 but undo the change that makes done_cond a separate field. We'll lose out on some niceness of the canonicalization but the trade-off doesn't seem worth it. We can revisit it in the future.

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.

ir::Builder::add_group() should take a done condition [IR] done condition should not be an assignment
1 participant