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

Working in simulation but incorrect synthesis output? #4

Open
bretthannigan opened this issue Mar 26, 2021 · 4 comments
Open

Working in simulation but incorrect synthesis output? #4

bretthannigan opened this issue Mar 26, 2021 · 4 comments

Comments

@bretthannigan
Copy link

bretthannigan commented Mar 26, 2021

The combinatorial LFSR module works perfectly in the following testbench:

`timescale 1ns/1ns
`include "../rtl/lfsr.v"
module lfsr_tb();

reg [7:0] data_in;
reg [31:0] state_in;
wire [7:0] data_out;
wire [31:0] state_out;

lfsr #(
    .LFSR_WIDTH(32),
    .LFSR_POLY(32'h4C11DB7),
    .LFSR_CONFIG("GALOIS"),
    .REVERSE(0),
    .DATA_WIDTH(8),
    .STYLE("REDUCTION")
) DUT (
    .data_in(8'h01),
    .state_in(32'hFFFFFFFF),
    .data_out(data_out),
    .state_out(state_out)
);

initial
begin
    state_in = 32'hFFFFFFFF;
    data_in = 8'h01;
end

initial
    #400 $stop;

initial 
begin
    $dumpfile("out.vcd");
    $dumpvars(0,DUT);
end

endmodule

I obtain the output state_out = 0x4AC9A203, which is correct compared to an online CRC-32 calculator. When I use the following code to synthesize and program an iCE40HX-8K device using the IceStorm toolchain, the output is different.

`include "lfsr.v"
module top (clk, o_led);

    input wire clk;
    output wire [7:0] o_led;
    wire [31:0] state;

    lfsr #(
        .LFSR_WIDTH(32),
        .LFSR_POLY(32'h4C11DB7),
        .LFSR_CONFIG("GALOIS"),
        .REVERSE(0),
        .DATA_WIDTH(8),
        .STYLE("REDUCTION")
    ) crc (
        .data_in(8'h01),
        .state_in(32'hFFFFFFFF),
        .data_out(),
        .state_out(state)
    );
    assign o_led = state[31:24];

endmodule

I now obtain state_out = 0x4E08BFB4 from the hardware. Interestingly, I get the correct results when .data_in(FF). Also (may be unrelated), synthesis does not complete with .STYLE("LOOP"), failing on the error:
ERROR: Conflicting init values for signal 1'0 (\crc.i [0] = 1'1, \crc.data_out_reg [0] = 1'0).

Has anyone got this to work correctly on this FPGA/toolchain?

@alexforencich
Copy link
Owner

Sounds like a yosys bug of some sort. Synplify also has issues. This code works in ISE, Vivado, Quartus Prime, and Quartus Prime Pro.

@bretthannigan
Copy link
Author

bretthannigan commented Mar 30, 2021

Okay, I have fixed the issue and it was quite simple. I removed the initialization to zero of state_val and data_val in lines 208 and 209 of lfsr.v. With this change the yosys iCE40 output matches simulation and an online CRC calculator. Next, to solve the issue of LOOP style not synthesizing, I made similar changes to lines 398 and 399 as well as defined new integer variables for i and j in the for loops as Yosys seems not to scope the loop indices correctly. Not sure why this fixes it but it works for me now.

@alexforencich
Copy link
Owner

Very odd. Well, at some point I need to look in to rewriting this module to use a constant function instead of the initial block so that synplify is happy. I have had several people complain about that, IMO it's a synplify bug as most other tools are perfectly fine with constant elaboration from initial blocks. I mainly need to make sure it that will still work in ISE. But if all that looks good, then presumably yosys will also be happy with that.

Forty-Bot added a commit to Forty-Bot/verilog-lfsr that referenced this issue Jan 2, 2023
Variable declaration assignments such as

	reg foo = 1;

are interpreted by some tools as

	reg foo;
	always @(*) foo = 1;

and not as

	reg foo;
	initial foo = 1;

This can cause simulation-synthesis mismatches, as the resultant value
of foo depends on how the tool is feeling.

Variable declaration assignments are used in four places. However, the
variables in question are subsequently assigned in their entirety,
Remove these assignments.

Closes: alexforencich#4
@alexforencich
Copy link
Owner

Alright, I think I finally have a rewrite done that uses constant functions instead of initial blocks, and also does some other reorganizing. Please let me know if it works any better in Yosys. I need to do some more testing, but so far it looks like it works in ISE, Vivado, Quartus Prime, and Quartus Prime Pro.

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

No branches or pull requests

2 participants