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

Implement combinational groups and primitives #635

Merged
merged 52 commits into from
Sep 7, 2021
Merged

Implement combinational groups and primitives #635

merged 52 commits into from
Sep 7, 2021

Conversation

rachitnigam
Copy link
Contributor

@rachitnigam rachitnigam commented Aug 27, 2021

This will be a relatively big PR with pervasive changes.

TODO

  • BREAKING: Deprecate non-comb groups attached to with clauses. This will require changes to the frontends.
  • comb groups will be converted into normal groups by the remove-comb-groups pass.
  • It will be an error to use combinational groups within normal control operators.
  • Make top-down-cc work again.
  • Write more tests from Generate size efficient FSMs in top-down-cc #639 for FSMs generated by top-down-cc.
    • Group that writes to the same register in a sequence.
    • if immediately nested in a while.
    • while as the first node in the control program
  • Fix frontends
  • Fix interpreter
  • Add tests for new passes
    • remove-dead-groups
      • Combinational groups
      • Normal groups
    • minimize-regs
      • Test that fails without the change of adding port in if and while to the gens list

Punt

Notes

Change to Language Semantics

Change the invariant for compilation passes: control nodes cannot have with attached to them anymore. This means that if-with and while-with need to be converted in simple if and while nodes. This will be done either in a new pass or remove-comb-groups:

New syntax for if without with:

manipulate_r_out; // This group modifies r.out
if r.out { ... } else { ... }

In this case, we need to be sure that r.out will have a stable value during the entire execution of either the true or false branch. For frontends, they can manipulate the value of r.out during the execution of the branches. The compiler must use the new pass to ensure that the value in the port remains stable. @sampsyo does this feel reasonable or am I making this unnecessarily expressive?

while w/o with will have a similar syntax and restrictions.

Normal if-with syntax will be modified to only allow for combinational groups in the with position. remove-comb-group can always ensure the stable property because combinational groups cannot be used in normal control flow.

Changes to Passes

remove-comb-groups

The pass will transform comb groups to normal groups using the original heuristic. The only change is that it will also hoist the group into a seq before the if or while: if lt.out with cond { .. } turns into: seq { cond; if lt_reg.out { .. }
The if transformation is:

cond0; // Runs assignments in cond and saves value of lt.out
if comb_reg.out { // register with value of lt.out
  ...
}

The while transformation is:

cond0;
while comb_reg.out {
  ...
  cond0;
}

top-down-cc

BREAKING: Requires top-down-cc to run before this and assumes there are no groups in with position. New mechanism to generate FSMs (see #639).


Invalid Notes

Changes that didn't make into this PR. I'm leaving them here for future context.

@stable annotation on conditional ports

The compilation passes will require ports used in the cond position to be marked with the annotation @stable which tells the pass that the port's value will not change during the execution of the branches of if or the body of a while loop. When compiling comb groups, remove-comb-groups will add this annotation. For non combinational conditions, the frontend must add this annotation.

Turns out we don't need the @stable stuff with the new FSM representation generated by top-down-cc.

(NOTE: I'm starting to think that this strategy will not work for while loops at all since they might need to recompute things for the condition and therefore need to run a cond group.)

@rachitnigam rachitnigam linked an issue Sep 2, 2021 that may be closed by this pull request
@rachitnigam rachitnigam changed the title Implement combinational groups, components, and primitives Implement combinational groups and primitives Sep 2, 2021
@rachitnigam
Copy link
Contributor Author

Speedups from the new, more-concise dynamic FSMs:

 ✗ correctness dynamic:tests/correctness/if.futil
         ~
    1   1│  {
    2    │-   "cycles": 12,
        2│+   "cycles": 11,
    3   3│    "memories": {
    4   4│      "mem": [
         ~
 correctness dynamic:tests/correctness/if-static-different-latencies.futil
         ~
    1   1│  {
    2    │-   "cycles": 12,
        2│+   "cycles": 11,
    3   3│    "memories": {
    4   4│      "i": [
         ~
 ✗ correctness dynamic:tests/correctness/while.futil
         ~
    1   1│  {
    2    │-   "cycles": 50,
        2│+   "cycles": 33,
    3   3│    "memories": {
    4   4│      "i": [
         ~
 ✗ correctness dynamic:tests/correctness/invoke-memory.futil
         ~
    1   1│  {
    2    │-   "cycles": 45,
        2│+   "cycles": 39,
    3   3│    "memories": {
    4   4│      "d": [
         ~
 ✗ correctness dynamic:tests/correctness/unsigned-dot-product.futil
         ~
    1   1│  {
    2    │-   "cycles": 60,
        2│+   "cycles": 55,
    3   3│    "memories": {
    4   4│      "mem0": [
         ~
 ✗ correctness dynamic:tests/correctness/pow.futil
         ~
    1   1│  {
    2    │-   "cycles": 107,
        2│+   "cycles": 96,
    3   3│    "memories": {
    4   4│      "base": [
         ~
 ✗ correctness dynamic:tests/correctness/invoke.futil
         ~
    1   1│  {
    2    │-   "cycles": 452,
        2│+   "cycles": 401,
    3   3│    "memories": {
    4   4│      "a0": [
         ~
 ✗ correctness dynamic:tests/correctness/pipelined-mac.futil
         ~
    1   1│  {
    2    │-   "cycles": 286,
        2│+   "cycles": 213,
    3   3│    "memories": {
    4   4│      "a": [
         ~

@rachitnigam
Copy link
Contributor Author

Hey @EclecticGriffin! Can I get some help fixing the interpreter? The changes to the normal interpreter seemed straightforward to make but the stepper changes seemed more involved.

@EclecticGriffin
Copy link
Collaborator

Sure thing, I'll untangle those

@rachitnigam
Copy link
Contributor Author

Thanks! Apart from the interpreter fixes, the PR is ready for review & merge.

@EclecticGriffin
Copy link
Collaborator

EclecticGriffin commented Sep 5, 2021

alright that should do it for the interpreter (probably). What's the situation with static-timing now since it seems to no longer be available? I adjusted the tests, but as it stands there are some redundant suites since we can't explicitly enable static timing anymore.

edit: ah just saw the punt on static timing. nvm then

sidebar: I am very much not looking forward to merging these interpreter changes with the multi-component stuff.

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.

Generate size efficient FSMs in top-down-cc Precise go/done interface
2 participants