chipsalliance / f4pga-examples

Example designs showing different ways to use F4PGA toolchains.
https://f4pga-examples.readthedocs.io
Apache License 2.0
263 stars 77 forks source link

Localparam not working properly #312

Open westonMS opened 2 years ago

westonMS commented 2 years ago

When the code was compiled using Surelog-yosys plugin, the bitstream was generated, but would not work properly. The State Machine would not cycle through, which caused each button on the counter to only work once until it was reset using the center button. This was caused by the use of localparam

This is probably caused by the parameters defaulting to one bit. Similar to this bug

The default Yosys parser has no problems with this.

Problem Code:

    localparam ZERO = 2'b00;
    localparam INC = 2'b01;
    localparam ONE = 2'b10;

Solution: Either changing each occurrences of ZERO, INC, and ONE with their respective values or using typedef enum to create a state type.

The bug appeared in OneShot.zip written by Professor Mike Wirthlin

rkapuscik commented 2 years ago

Thanks for reporting, we will investigate this.

westonMS commented 2 years ago

After further investigation it appears the problem is not that it defaults to one bit, but that in a case statement the parameter is not handled correctly if it is multiple bits wide. In OneShot.sv, if ONE: in the case statement is replaced by 2'b10: the code functions properly.

Zach227 commented 1 year ago

I have done further testing with this simplified design: test.sv, test.xdc

It looks like this bug is caused by the way that Yosys handles the UHDM model of the design. Parsing the design directly in yosys (using the versions that are installed in the conda environment) and printing the AST dump resulted in:

always @*
        begin
          led = 8'b 10101010;
          case (sw)
            0:
              led = 8'b 00000010;
            1:
              led = 8'b 00000111;
            -2:
              led = 8'b 00111100;
            -1:
              led = 8'b 11111111;
          endcase
        end

It processes the localparams as two's compliment so when even when sw = 2'b10 it does not match the value of -2 that is expected. I manually installed Yosys and the UHDM plugin on my machine using the yosys-systemverilog repo as a reference. When I parsed it again using this version, the AST dump was different.

always @*
        begin
          led = 8'b 10101010;
          case (sw)
            2'b 00:
              led = 8'b 00000010;
            2'b 01:
              led = 8'b 00000111;
            2'b 10:
              led = 8'b 00111100;
            2'b 11:
              led = 8'b 11111111;
          endcase
        end

I then replaced yosys from my f4pga install with this new version of Yosys. I replaced it in opt/f4pga/xc7/conda/envs/xc7/share and opt/f4pga/xc7/conda/envs/xc7/bin. When I ran the tool-chain with these changes the design worked correctly.

wsipak commented 1 year ago

As Zach227 has observed, this can be fixed by upgrading the tools. With Yosys 0.27+22 (git sha1 0f5e7c244) and yosys-f4pga-plugins (git sha1 e7070ca645), which are used here: https://github.com/antmicro/yosys-systemverilog I'm getting the expected output for OneShot.sv. I think the issue can be closed.