calyxir / calyx

Intermediate Language (IL) for Hardware Accelerator Generators
https://calyxir.org
MIT License
501 stars 52 forks source link

`resource-sharing`: Sharing generates invalid verilog #532

Open rachitnigam opened 3 years ago

rachitnigam commented 3 years ago

The unrolled MVT benchmarks generates invalid verilog:

%Warning-UNOPTFLAT: /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1076:18: Signal unoptimizable: Feedback to clock or circular logic: 'main.add9_right'
 1076 |     logic [31:0] add9_right;
      |                  ^~~~~~~~~~
                    ... Use "/* verilator lint_off UNOPTFLAT */" and lint_on around source to disable this message.
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1076:18:      Example path: main.add9_right
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:750:14:      Example path: ASSIGNW
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1077:18:      Example path: main.add9_out
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:4700:24:      Example path: ASSIGNW
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1067:18:      Example path: main.add10_right
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:750:14:      Example path: ASSIGNW
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1068:18:      Example path: main.add10_out
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:4706:24:      Example path: ASSIGNW
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1070:18:      Example path: main.add11_right
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:750:14:      Example path: ASSIGNW
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1071:18:      Example path: main.add11_out
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:4712:24:      Example path: ASSIGNW
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1073:18:      Example path: main.add12_right
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:750:14:      Example path: ASSIGNW
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1074:18:      Example path: main.add12_out
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:4718:23:      Example path: ASSIGNW
                    /var/folders/sk/f158_qn147vc0jhjsfdcswq40000gn/T/tmpweji9c61:1076:18:      Example path: main.add9_right
%Error: Exiting due to 1 warning(s)
rachitnigam commented 3 years ago

Looks like the resource-sharing pass causes this problem. Running the benchmark with -d resource-sharing generates valid verilog and generates the right values.

rachitnigam commented 3 years ago

Minimal test case:

import "primitives/core.futil";

component main(@go go: 1, @clk clk: 1, @reset reset: 1) -> (@done done: 1) {
  cells {
    add10 = std_add(32);
    add7 = std_add(32);
    add8 = std_add(32);
    add9 = std_add(32);

    r1 = std_reg(32);
    r2 = std_reg(32);
  }
  wires {
    group upd33 {
      add7.left = 32'd0;
      add7.right = 32'd0;
      add8.left = 32'd0;
      add8.right = add7.out;
      r1.write_en = 1'd1;
      r1.in = add8.out;
      upd33[done] = r1.done ? 1'd1;
    }
    group upd34 {
      add9.left = 32'd0;
      add9.right = 32'd0;
      add10.left = 32'd0;
      add10.right = add9.out;
      r2.write_en = 1'd1;
      r2.in = add10.out;
      upd34[done] = r2.done ? 1'd1;
    }
  }

  control {
    seq {
      upd33;
      upd34;
    }
  }
}
rachitnigam commented 3 years ago

https://github.com/verilator/verilator/issues/3146 could be the potential problem

rachitnigam commented 3 years ago

Argh, resource sharing might be a fundamentally broken pass. It might only make sense to share sequential components.

rachitnigam commented 3 years ago

@sampsyo @EclecticGriffin and I discussed this problem and we agree that the generated code is "valid" in that the combinational loop is stable. However, we need to do some digging before we can decide what the right steps are here.

Two things to do:

  1. See what vivado does when you give it this program
  2. See if the verilator can simulate this problem when the UNOPTFLAT warning is disabled.