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

resource-sharing: Sharing generates invalid verilog #532

Open
rachitnigam opened this issue Jun 8, 2021 · 5 comments
Open

resource-sharing: Sharing generates invalid verilog #532

rachitnigam opened this issue Jun 8, 2021 · 5 comments
Labels
C: Calyx Extension or change to the Calyx IL S: Available Can be worked upon Type: Bug Bug in the implementation

Comments

@rachitnigam
Copy link
Contributor

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 rachitnigam added the Type: Bug Bug in the implementation label Jun 15, 2021
@rachitnigam
Copy link
Contributor Author

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 rachitnigam changed the title Polybench unrolled MVT generates invalid verilog Bug in resource sharing: polybench-unrolled-mvt generates invalid verilog Jul 30, 2021
@rachitnigam rachitnigam added the C: Calyx Extension or change to the Calyx IL label Jul 30, 2021
@rachitnigam rachitnigam changed the title Bug in resource sharing: polybench-unrolled-mvt generates invalid verilog resource-sharing: polybench-unrolled-mvt generates invalid verilog Aug 6, 2021
@rachitnigam rachitnigam changed the title resource-sharing: polybench-unrolled-mvt generates invalid verilog resource-sharing: Sharing generates invalid verilog Sep 22, 2021
@rachitnigam
Copy link
Contributor Author

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
Copy link
Contributor Author

verilator/verilator#3146 could be the potential problem

@rachitnigam
Copy link
Contributor Author

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

@rachitnigam
Copy link
Contributor Author

@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.

@rachitnigam rachitnigam added the S: Available Can be worked upon label Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Calyx Extension or change to the Calyx IL S: Available Can be worked upon Type: Bug Bug in the implementation
Projects
None yet
Development

No branches or pull requests

1 participant