Verilog: Assignment not working as expected

Question

I'm working on building a simple processor in Verilog. I'm now implementing the branch related instructions, but I'm observing some wrong (or at least unexpected) behavior. When I reach a branch instruction and the branch is taken, I should immediately update my Program Counter (PC) register to the branch destination.

Here is the code implementing the address selection:

always @* begin
 next = (branch_taken) ? branch_destination : next_pc + 4;
end
always @(posedge clock) begin
 if (reset) begin
 next_pc <= 0;
 pc <= 0;
 end
 else begin
 next_pc <= next;
 pc <= next_pc;
 end
end
always @(posedge clock) $display("pc: %h next_pc: %h next: %h branch_taken: %b clk: %d",
 pc, next_pc, next, branch_taken, clock);

I've created a testbench containing one branch (which should make branch_taken true) as the first instruction (at PC = 0). Here is the output:

pc: 00000000 next_pc: 00000000 next: 00000004 branch_taken: 0 clk: 0
pc: 00000000 next_pc: 00000004 next: 00000ff0 branch_taken: 1 clk: 1
pc: 00000004 next_pc: 00000ff0 next: 00000ff4 branch_taken: 0 clk: 2
pc: 00000ff0 next_pc: 00000ff4 next: 00000ff8 branch_taken: 0 clk: 3

Looking at the state changes, I can see that next does get updated to the right address within the first clock, but next_pc still gets updated to next_pc + 4 (00000004) instead of 00000ff0. If I use always @(negedge clock) for the pc update, I get the expected behaviour.

Why does next_pc not get the last value assigned to next?

• dave_59: Displaying your output with always @* is very misleading. You should change it to always @(posedge clk) $display(...). Then you will see the RHS values of the assignment being taken.
• TheGMX: Thanks @dave_59 ! I've used always @* to see if it changed branch_takenwithin the first clock, but reading your suggestion (and making the change) it looks clearer now. I've updated the original post accordingly.
• TheGMX: Thanks, @toolic. I did not run away. I was still investigating the issue, following the lead of Dave's last comment. Anyway, I now found that his suggestion was indeed accurate, and this question is indeed solved.

Answer 1

Why does next_pc not get the last value assigned to next?

But it does, in every case.

By having both next_pc and pc as clocked registers, you've created a 2-stage pipeline. One consequence of this is that you now have a "branch delay slot" following each branch instruction. The instruction in that slot (location 00000004 in your example) gets executed regardless of whether the branch is taken.

If this is not what you intended, then next_pc needs to be combinatorial, not a register.

• TheGMX: Hmm, that makes sense! Thanks @dave-tweed! The 2-stage pipeline was not intended. Still, I'm not sure I really understand why next is not assuming the value 00000ff0 right in the first clock cycle. I've looked at other implementations, and they proceed as I did, but for the same testbench, they do get next_pc to be 00000ff0 in the first clock. It looks like somehow my branch_taken does not get updated within the first cycle.
• Dave Tweed: Your printout is showing that branch_taken is not asserted until the second clock edge. You didn't show us the code that generates that, so I can't comment on it, but I'm guessing that there's another register involved.
• TheGMX: This was the issue. branch_taken was set upon instruction decode, but the memory was synchronous, so it took one cycle to decode it and another to set it. Anyway, your tips led me to uncover other problems, and I've ended up solving the issue by embracing the "branch delay slot" and an NOP. Many thanks, @dave-tweed!

Answer 2

You have a simulation race condition because you are trying to display signal values when they are changing. Your code changes pc at the posedge of the clock, and the code also calls $display at the same clock edge. You may get the value of pc before or after it changed; the simulator is not guaranteed to show one or the other.

If I use always @(negedge clock) for the pc update, I get the expected behaviour.

If you call $display on the posedge still, then you will be showing the signals values when they are stable, as desired.

Since you want your logic to be sensitive to posedge, change the display code to the negedge:

always @(negedge clock) $display("pc: %h next_pc: %h next: %h branch_taken: %b clk: %d",
 pc, next_pc, next, branch_taken, clock);