-1

I am recieving some strange results when trying to compile and simulate a Verilog module and stimulus. If I simulate it in Silos, the code functions as expected. If I simulate it in Icarus (iverlog and vvp) the time differs from Silos(the starting at 0 rather than 200 I don't care about as much as Silos has 235 -> 255 and Icarus has 235 -> 265). The Silos repeat function works as I would expect, but when using Icarus I can't really seem to figure out how they got that result. Also, when changing the repeat R2GDELAY to 3, Icarus also does not seem to preform as expected. Is there something I am missing when using Icarus such as I must manually set the start time to 0 for an accurate result later in the simulation, or Silos auto-initializes variables which I must do manually in Icarus? This code is taken form a Verilog HDL book which can be found here http://authors.phptr.com/palnitkar/

Here is the code:

`define TRUE    1'b1
`define FALSE   1'b0
`define RED     2'd0
`define YELLOW  2'd1
`define GREEN   2'd2

//State definition   HWY          CNTRY
`define S0  3'd0  //GREEN          RED
`define S1  3'd1  //YELLOW         RED
`define S2  3'd2  //RED            RED
`define S3  3'd3  //RED            GREEN
`define S4  3'd4  //RED            YELLOW

//Delays
`define Y2RDELAY    3   //Yellow to red delay
`define R2GDELAY    2   //Red to Green Delay

module sig_control (hwy, cntry, X, clock, clear);

//I/O ports
output [1:0] hwy, cntry;
            //2 bit output for 3 states of signal
            //GREEN, YELLOW, RED;
reg [1:0] hwy, cntry;
            //declare output signals are registers

input X;
            //if TRUE, indicates that there is car on
          //the country road, otherwise FALSE
input clock, clear;
//Internal state variables
reg [2:0] state;
reg [2:0] next_state;


initial
    begin
        state = `S0;
        next_state = `S0;
        hwy = `GREEN;
        cntry = `RED;
    end

//state changes only at positive edge of clock
always @(posedge clock)
    state = next_state;

//Compute values of main signal and country signal
always @(state)
    begin
        case(state)
            `S0: begin
                    hwy = `GREEN;
                    cntry = `RED;
                end
            `S1: begin
                    hwy = `YELLOW;
                    cntry = `RED;
                end
            `S2: begin
                    hwy = `RED;
                    cntry = `RED;
                end
            `S3: begin
                    hwy = `RED;
                    cntry = `GREEN;
                end
            `S4: begin
                    hwy = `RED;
                    cntry = `YELLOW;
                end
    endcase
end


//State machine using case statements
always @(state or  X)
    begin
        if(clear)
            next_state = `S0;
        else
            case (state)
                `S0: if(X)
                        next_state =  `S1;
                    else
                        next_state =  `S0;
                `S1: begin //delay some positive edges of clock
                        repeat(`Y2RDELAY) @(posedge clock) ;
                        next_state =  `S2;
                    end
                `S2: begin //delay some positive edges of clock
                        //EDIT ADDED SEMICOLON
                        repeat(`R2GDELAY) @(posedge clock);
                        next_state =  `S3;
                    end
                `S3: if( X)
                        next_state =  `S3;
                    else
                        next_state =  `S4;
                `S4: begin //delay some positive edges of clock
                        repeat(`Y2RDELAY) @(posedge clock) ;
                        next_state =  `S0;
                    end
            default: next_state =  `S0;
        endcase
    end
endmodule

//Stimulus Module
module stimulus;

wire [1:0] MAIN_SIG, CNTRY_SIG;
reg CAR_ON_CNTRY_RD;
            //if TRUE, indicates that there is car on
          //the country road
reg CLOCK, CLEAR;

//Instantiate signal controller
sig_control SC(MAIN_SIG, CNTRY_SIG, CAR_ON_CNTRY_RD, CLOCK, CLEAR);


//Setup monitor
initial
    $monitor($time, " Main Sig = %b Country Sig = %b Car_on_cntry = %b",
                                            MAIN_SIG, CNTRY_SIG, CAR_ON_CNTRY_RD);
//setup clock
initial
    begin
        CLOCK = `FALSE;
        forever #5 CLOCK = ~CLOCK;
    end

//control clear signal
initial
    begin
        CLEAR = `TRUE;
        repeat (5) @(negedge CLOCK);
        CLEAR = `FALSE;
    end

//apply stimulus
initial
    begin
        CAR_ON_CNTRY_RD = `FALSE;

        #200 CAR_ON_CNTRY_RD = `TRUE;
        #100 CAR_ON_CNTRY_RD = `FALSE;

        #200 CAR_ON_CNTRY_RD = `TRUE;
        #100 CAR_ON_CNTRY_RD = `FALSE;

        #200 CAR_ON_CNTRY_RD = `TRUE;
        #100 CAR_ON_CNTRY_RD = `FALSE;

        #100 $finish;
    end
endmodule

Here is the output from Silos:

             200 Main Sig = 10 Country Sig = 00 Car_on_cntry = 1
             205 Main Sig = 01 Country Sig = 00 Car_on_cntry = 1
             235 Main Sig = 00 Country Sig = 00 Car_on_cntry = 1
             255 Main Sig = 00 Country Sig = 10 Car_on_cntry = 1
             300 Main Sig = 00 Country Sig = 10 Car_on_cntry = 0
             305 Main Sig = 00 Country Sig = 01 Car_on_cntry = 0
             335 Main Sig = 10 Country Sig = 00 Car_on_cntry = 0
             500 Main Sig = 10 Country Sig = 00 Car_on_cntry = 1
             505 Main Sig = 01 Country Sig = 00 Car_on_cntry = 1
             535 Main Sig = 00 Country Sig = 00 Car_on_cntry = 1
             555 Main Sig = 00 Country Sig = 10 Car_on_cntry = 1
             600 Main Sig = 00 Country Sig = 10 Car_on_cntry = 0
             605 Main Sig = 00 Country Sig = 01 Car_on_cntry = 0
             635 Main Sig = 10 Country Sig = 00 Car_on_cntry = 0
             800 Main Sig = 10 Country Sig = 00 Car_on_cntry = 1
             805 Main Sig = 01 Country Sig = 00 Car_on_cntry = 1
             835 Main Sig = 00 Country Sig = 00 Car_on_cntry = 1
             855 Main Sig = 00 Country Sig = 10 Car_on_cntry = 1
             900 Main Sig = 00 Country Sig = 10 Car_on_cntry = 0
             905 Main Sig = 00 Country Sig = 01 Car_on_cntry = 0
             935 Main Sig = 10 Country Sig = 00 Car_on_cntry = 0

Here is the output from iverilog:

               0 Main Sig = 10 Country Sig = 00 Car_on_cntry = 0
             200 Main Sig = 10 Country Sig = 00 Car_on_cntry = 1
             205 Main Sig = 01 Country Sig = 00 Car_on_cntry = 1
             235 Main Sig = 00 Country Sig = 00 Car_on_cntry = 1
             265 Main Sig = 00 Country Sig = 10 Car_on_cntry = 1
             300 Main Sig = 00 Country Sig = 10 Car_on_cntry = 0
             305 Main Sig = 00 Country Sig = 01 Car_on_cntry = 0
             335 Main Sig = 10 Country Sig = 00 Car_on_cntry = 0
             500 Main Sig = 10 Country Sig = 00 Car_on_cntry = 1
             505 Main Sig = 01 Country Sig = 00 Car_on_cntry = 1
             535 Main Sig = 00 Country Sig = 00 Car_on_cntry = 1
             565 Main Sig = 00 Country Sig = 10 Car_on_cntry = 1
             600 Main Sig = 00 Country Sig = 10 Car_on_cntry = 0
             605 Main Sig = 00 Country Sig = 01 Car_on_cntry = 0
             635 Main Sig = 10 Country Sig = 00 Car_on_cntry = 0
             800 Main Sig = 10 Country Sig = 00 Car_on_cntry = 1
             805 Main Sig = 01 Country Sig = 00 Car_on_cntry = 1
             835 Main Sig = 00 Country Sig = 00 Car_on_cntry = 1
             865 Main Sig = 00 Country Sig = 10 Car_on_cntry = 1
             900 Main Sig = 00 Country Sig = 10 Car_on_cntry = 0
             905 Main Sig = 00 Country Sig = 01 Car_on_cntry = 0
             935 Main Sig = 10 Country Sig = 00 Car_on_cntry = 0

EDIT: Added semicolon as indicated in the code above.
Thanks for your help!

dannyn382
  • 363
  • 2
  • 5
  • 15

1 Answers1

4

You have race conditions in your logic, as mentioned in the comments you should not be using the clock as an input to your combinational logic.

Review the following two logic blocks:

1. repeat(`R2GDELAY) @(posedge clock)
     next_state =  `S3;

2. always @(posedge clock)
     state = next_state;

When posedge clock occurs, a simulator will chose one of these two statements to execute first, with no rules as to which it might choose. If it chooses #1 first, the next state will be set to S3, and then #2 will execute, assigning state to S3. If #2 executes first, state will be set to something else, and then next_state will be set to S3 after the state is assigned.

Now you have divergent behavior based on which random event was chosen to execute first by the simulator.

The way to avoid this is not to have your combinational blocks look at the clock in any way. The clock should only be used to update your registers, with nonblocking assignments <=.

Tim
  • 35,413
  • 11
  • 95
  • 121
  • Nicely spotted - I marked up errors in Palnitkar when I first read it, but I gave up. Just too many of them. – EML Nov 29 '13 at 13:26
  • Thank you for your comments. To achieve the expected behavior while avoiding the race condition, are you suggesting that I create additional states instead of using repeat? Also do you know why the author wrote the state machine using repeats if that creates a race condition? Thanks again! – dannyn382 Nov 30 '13 at 03:36
  • You don't have to create additional states, but you could do more with the current states. When you enter S1 you can have a counter start counting down each clock cycle until it reaches 0, and when the state is S1 and the counter is zero, then you move to state S2, etc. @dannyn382 – Tim Nov 30 '13 at 04:06
  • Above I have edited the code to include a semicolon after the repeat ('R2GDELAY) @ (posedge clock); as it was a typo in the version of book I had. The newer version includes a semicolon in that spot. Would you be able to explain how that effects your response? @Tim – dannyn382 Dec 02 '13 at 22:03