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

Localparam not working properly #312

Open
westonMS opened this issue May 31, 2022 · 4 comments
Open

Localparam not working properly #312

westonMS opened this issue May 31, 2022 · 4 comments

Comments

@westonMS
Copy link

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

Thanks for reporting, we will investigate this.

@westonMS
Copy link
Author

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

Zach227 commented Feb 8, 2023

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

wsipak commented Apr 6, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants