1-instruction delay of branching in Verilog implementation and syncing issue

Question

I have a 16-bit single-cycle, very sparse MIPS implementation that I've been working on in Verilog. Everything works except for the fact that branching is delayed by one entire clock cycle.

always @(posedge clock) begin
    // Necessary to add this in order to ensure PC => PC_next
    iaddr <= pc_next 
end

The above code is used to update the program counter/instruction address, which comes from a module, PCLogic:

module PCLogic(
        pc_next,    // next value of the pc
        pc,     // current pc value
        signext,    // from sign extend circuit
        branch, // beq instruction
        alu_zero,   // zero from ALU, used in cond branch
        reset       // reset input
        );

output [15:0] pc_next;
input [15:0] pc;
input [15:0] signext;  // From sign extend circuit
input branch;
input alu_zero;
input reset;

reg [15:0] pc_next; 

    always @(pc or reset) begin
        if (reset == 1)
            pc_next = 0;
        else if (branch == 1 && alu_zero == 1)
            pc_next = pc+2+(signext << 1);
        else
            pc_next = pc+2;
    end

endmodule

iaddr is a simple 16-bit register that stores the program counter.

I don't understand why there might be a problem with this circuit, but for some reason, the entire circuit is delayed by a single clock cycle until it branches (e.g. if I have a BEQ instruction at 0x16 that always jumps, it will execute the next instruction at 0x18 and then jump to the relative offset, but from 0x20).

I can almost feel like the solution is right in front of me but I don't know what I'm missing about the semantic. The offset problem is solved if I remove the +2 that's always implicit unless there is a true "bubble" or hardware-induced no-op, but the delay is still present.

Can someone explain to me what causes the delay and why it happens?

The logic states that whenever `pc` changes and the *anding* of `branch` with `alu_zero` is *zero*, then `pc_next` must be incremented by `2`. From the snippet, I guess `alu_zero` or `branch` is not updated properly. Can you share some pseudo-testbench or waveform for this? — sharvil111, Dec 02 '15 at 04:05
@sharvil111 Actually, I've just solved this problem! I'll post an answer to my own question. — CinchBlue, Dec 02 '15 at 04:10
**Signals read inside a combinational always block must be present in the sensitivity list. It's a coding rule so that the synthesis tool can convert it into actual logic gates.** thanks @sharvil111 — CinchBlue, Dec 02 '15 at 05:06

score 2 · Accepted Answer · answered Dec 02 '15 at 04:12

The answer is that using state within the PCLogic module will cause an additional propagation delay. By removing a register in PCLogic, we remove an implicit state step in the module itself, reducing its propagation down to negligibly 0.

So the answer is to change pc_next to be computed by an always @(pc) block to one based on declarative expressions:

wire [15:0] pc_next = (reset == 1)? 0 : (branch == 1 && alu_zero == 1)? pc+2+(signext << 1) : pc+2;

By changing our circuit into a combinatoric circuit, we no longer need to store state and thus reduce a "buffer" in our process. The PC can now update in just (T) time instead of (2T).

score 2 · Answer 2 · edited May 23 '17 at 12:08

2

Another way to code a combinational circuit:

reg [15:0] pc_next; 

always @* begin
    if (reset == 1)
        pc_next = 0;
    else if (branch == 1 && alu_zero == 1)
        pc_next = pc+2+(signext << 1);
    else
        pc_next = pc+2; // latch will be inferred without this
end

You'll be needing this when your combinational circuit gets more complex since assign statements are difficult to read when there are a lot of nested if-else.

Note about this

pc_next = pc+2; // latch will be inferred without this

A combinational block should have a default value. When there is no default value defined in a conditional statement, it will retain its value and lead to an incorrect behavior. A combinational block must not hold a value.

For more information about unexpected latch see this.

edited May 23 '17 at 12:08

Community

1
1

answered Dec 02 '15 at 04:35

e19293001

2,783
9
42
54

This was how I originally coded the circuit. It doesn't work if it make it this way. But wait... does the `always @(*)` change things? Or is this essentially the same? What was the difference between this and the code that I had originally? – CinchBlue Dec 02 '15 at 04:40
`always @(*)` is sensitive to all input signals. This would be the same but more readable for complex circuits especially when there are lots of nested if-else statement. – e19293001 Dec 02 '15 at 04:45
1

Sensitivity list must be specified for all of the input signals of your combinational block. In your code, the sensitivity list must be `always @(pc, reset, alu_zero, branch, signext)` since those are all your input signals for your combinational circuit. To simplify, just use `always @*` for all combinational block. – e19293001 Dec 02 '15 at 04:50
Just make sure you define a default value to avoid latches that cause unintended behavior – e19293001 Dec 02 '15 at 04:52
Wow that's really strange – CinchBlue Dec 02 '15 at 04:53
For some reason, if I reduce the sensitivity list to just `(pc or reset)`, it doens't work and has the behavior of the original circuit. Do you have any idea why? – CinchBlue Dec 02 '15 at 04:55
You should define all your input signals to the sensitivity list. `branch` and `alu_zero` are input signals of your combinational circuit too. – e19293001 Dec 02 '15 at 05:01
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/96757/discussion-between-e19293001-and-vermillionazure). – e19293001 Dec 02 '15 at 05:01
@ e19293001 and @VermillionAzure Just a clarification. `always @(pc or reset)` will execute only when `reset` or `pc` changes. While `always @(*)` may also execute when `branch` or `alu_zero` changes. The use of either depends on the OPs intent. – sharvil111 Dec 02 '15 at 05:34
@sharvil111 I've also noticed that the circuit works correctly if I put in every input intot he sensitivity list except `branch`. It's a bit strange. – CinchBlue Dec 02 '15 at 05:35
`branch` must also be included in your sensitivity list. – e19293001 Dec 02 '15 at 05:39
You could just use `always @*` instead? – e19293001 Dec 02 '15 at 05:42

1-instruction delay of branching in Verilog implementation and syncing issue

2 Answers2