Converting finite state machine diagram to Verilog code

Question

I need to convert the following finite state diagram into Verilog code.

I've included the code I've written so far below. It looks to me like I've implemented all of the logic correctly and the code works for the first few input combinations. It eventually fails however and I can't seem to figure out why.

module FiniteStateMachine(output reg out_z, input in_x, in_y, clk, reset_b);

    parameter   S0 = 2'b00, S1 = 2'b01, S2 = 2'b10, S3 = 2'b11;
    reg         state;

    always @(posedge clk, negedge reset_b) begin

        // set state
        if (reset_b || !in_x) state <= S0;
        else
            case (state)
                S0: state <= (in_y == 1) ? S1 : S3;
                S1: state <= S2;
                S2: state <= S3;
                S3: state <= S3;
            endcase

        // set output
        out_z <= (state == S2 || state == S3) ? 1 : 0;

    end

endmodule

Answer 1

There are actually a number of problems with your implementation as it is now:

1. Your state variable is only one bit wide when it needs to be two: reg state -> reg [1:0] state
2. Its likely that you dont actually want to have out_z be a register as it should follow the state on the same clock, not a clock cycle after.
3. Your reset_b logic is backwards, for negedge resets, you need to checkfor assertion via !reset_b or ~reset_b.
4. As has been mentioned, you really shouldn't combine asynchronous resets with any synchronous input, even a synchronous reset, as you have done with reset_b and in_x. While this will work fine in simulation, most synthesis tools will likely be unable to handle it properly. When learning Verilog, I made this same mistake and it took days to discover and correct.

Heres a cleaner version of your code with this fixes implemented and commented so you can see these 4 points:

module FiniteStateMachine(output reg out_z, input in_x, in_y, clk, reset_b);

  parameter S0 = 2'b00, S1 = 2'b01, S2 = 2'b10, S3 = 2'b11;
  reg [1:0] state; // Fix state variable

  // Set output combinationally (no need for turning operator)
  always @(*) begin
    out_z = (state == S2 || state == S3);
  end

  always @(posedge clk, negedge reset_b) begin
    // Invert the logic for reset and keep it separate
    if (!reset_b) begin
      state <= S0;
    end
    else begin
      // You can case on inputs as was suggested, but I think casing on state is fine
      // I include only logic for changing state
      case (state)
        S0: begin
          if (in_x && in_y) begin
            state <= S1;
          end
          else if (in_x && !in_y) begin
            state <= S3;
          end
        end
        S1: begin
          if (in_x) begin
            state <= S2;
          end
          else begin
            state <= S0;
          end
        end
        S2:  begin
          if (in_x) begin
            state <= S3;
          end
          else begin
            state <= S0;
          end
        end
        S3: begin
          if (!in_x) begin
            state <= S0;
          end
        end
      endcase
    end
  end

endmodule