Electrical – Possible issue with vivado synthesis encoding state machines

I have been working on using the ethernet phy on my Nexys4 DDR for the last few weeks. Over the last few days I have been particularly frustrated with one issue I was having with my rx module. I have pasted the code bellow. My module goes through three different states. Waiting for preamble, signaling the start of frame, and receiving the body of the packet. These three states are stored using the state register. The design works fine in the simulator and when I first tested it on the card.

To test it I have been using chipscope and I had reg state marked as debug. Then I removed the chip scope probes and everything started to fail. It would signal the start of frame and then nothing else. Through experimentation I found that when I have a probe on the state register the whole design works, but if I remove that probe the design falls apart. At first this seemed like a timing issue since the eth_* signals come from the ethernet phy directly (I drive the phy with a clock 45 degrees out of phase from clk_mac.) I added a pipeline stage between those signals and my combination logic and it was still failing. Now I don't think it is a timing issue since it is unlikely that I would get a valid preamble and first byte if those signals were wrong.

After some more experimentation I noticed the following in the synthesis log….

    --------------------------------------------------------------------------------------------------
                       State |                     New Encoding |                Previous Encoding 
    ---------------------------------------------------------------------------------------------------
              STATE_PREAMBLE |                                0 |                              000
                   STATE_SOF |                                1 |                              001
    ---------------------------------------------------------------------------------------------------

INFO: [Synth 8-3354] encoded FSM with state register 'state_reg' using encoding 'sequential' in module 'eth_rx'

It appears that vivado is optimizing out the upper bits of the state register. (Note: before someone points it out I know I should probably only have [1:0] instead of [2:0] but my design started out with more states than I ended up needing and I haven't changed it yet.) Either way vivado is removing STATE_RECV which requires the state[1] bit to be set. By adding the chipscope I was preventing vivado from doing what seems like an illegal optimization. I think this is a bug with vivado. Has anyone else seen this or is there something wrong you see with my verilog? I am using vivado 2018.1.

`timescale 1 ns / 1 ps
module eth_rx
(
    input             clk_mac,
    input             rst_n,

    input             eth_crsdv,
    input             eth_rxerr,
    input  [1:0]      eth_rxd,

    (* mark_debug = "true" *)
    output reg        rx_vld,
    (* mark_debug = "true" *)
    output reg [7:0]  rx_dat,
    (* mark_debug = "true" *)
    output reg        rx_sof,
    (* mark_debug = "true" *)
    output reg        rx_eof,
    (* mark_debug = "true" *)
    output reg [10:0] rx_len,
    (* mark_debug = "true" *)
    output reg        rx_err
);

    `include "util.vh"

    (* mark_debug = "true" *)
    reg       eth_crsdv_b;
    (* mark_debug = "true" *)
    reg [1:0] eth_rxd_b;
    (* mark_debug = "true" *)
    reg       eth_rxerr_b;
    always @(posedge clk_mac) begin
        eth_crsdv_b <= eth_crsdv;
        eth_rxd_b   <= eth_rxd;
        eth_rxerr_b <= eth_rxerr;
    end

    (* mark_debug = "true" *)
    reg [63:0] data_buffer;
    reg [63:0] next_data_buffer;
    always @(posedge clk_mac) begin
        if(rst_n)
            data_buffer <= next_data_buffer;
        else
            data_buffer <= 0;
    end


    reg [10:0] frame_idx, next_frame_idx;
    reg [1:0]  dibit_cnt, next_dibit_cnt;
    always @(posedge clk_mac) begin
        dibit_cnt <= next_dibit_cnt;
        frame_idx <= next_frame_idx;
    end

    reg        next_rx_vld;
    reg        next_rx_sof;
    reg        next_rx_eof;
    reg [10:0] next_rx_len;
    reg [7:0]  next_rx_dat;
    reg        next_rx_err;
    always @(posedge clk_mac) begin
        rx_vld <= next_rx_vld;
        rx_dat <= next_rx_dat;
        rx_sof <= next_rx_sof;
        rx_eof <= next_rx_eof;
        rx_len <= next_rx_len;
        rx_err <= next_rx_err;
    end

    localparam STATE_PREAMBLE = 0;
    localparam STATE_SOF      = 1;
    localparam STATE_RECV     = 2;

    reg [2:0] state;
    reg [2:0] next_state;
    always @(posedge clk_mac) begin
        if(rst_n)
            state <= next_state; 
        else
            state <= STATE_PREAMBLE;
    end

    wire [31:0] crc32_code;
    crc32 crc32_inst
    (
        .clk (clk_mac),
        .rst (next_rx_sof && next_rx_vld),
        .vld (next_rx_vld && !next_rx_eof),
        .data(next_rx_dat),
        .crc (crc32_code)
    );

    always @* begin
        next_state  = state;
        next_rx_vld = 0;
        next_rx_eof = 0;
        next_rx_sof = 0;
        next_rx_err = 0; 
        next_rx_len = rx_len;
        next_rx_dat = rx_dat;

        next_frame_idx = frame_idx;
        next_dibit_cnt = dibit_cnt + 1;

        if(eth_crsdv_b && !eth_rxerr_b) 
            next_data_buffer = {eth_rxd_b, data_buffer[63:2]};
        else
            next_data_buffer = 0;

        case(state)
            STATE_PREAMBLE: begin
                next_dibit_cnt = 0;
                next_frame_idx = 0;
                if(data_buffer == 64'hd555555555555555) begin
                    next_state  = STATE_SOF;
                end
            end STATE_RECV, STATE_SOF: begin
                if(eth_rxerr_b) begin
                    next_rx_vld = 1;
                    next_rx_eof = 1;
                    next_rx_vld = 1;
                    next_state  = STATE_PREAMBLE; 
                end else if(eth_crsdv_b) begin
                    if(&next_dibit_cnt) begin
                        next_frame_idx = frame_idx + 1;
                        if(frame_idx >= 4) begin
                            next_rx_len = frame_idx - 3;
                            next_rx_vld = 1;
                            next_rx_dat = next_data_buffer[31:24];
                            next_rx_sof = state == STATE_SOF;
                            next_state  = STATE_RECV;
                        end
                    end
                end else begin
                    next_state  = STATE_PREAMBLE;
                    next_rx_eof = 1;
                    next_rx_vld = 1;
                    next_rx_err = crc32_code != bswap32(data_buffer[63:32]);
                end
            end
        endcase
    end

endmodule

UPDATE: I got rid of the FSM extraction and now it works fine…

localparam STATE_PREAMBLE = 3'b001;
localparam STATE_SOF      = 3'b010;
localparam STATE_RECV     = 3'b100;

//Override FSM because Vivado ignores STATE_RECV with auto
(* fsm_encoding = "none" *)
reg [2:0] state = STATE_PREAMBLE, next_state;
always @(posedge clk_mac) begin
    if(rst_n)
        state <= next_state; 
    else
        state <= STATE_PREAMBLE;
end