blocking assignment must be used for a clock divider

[hirooih]

Let me propose this again. It gets longer, but I hope this let you understand.

---

We have the following rule;

Do not use #delay in synthesizable design modules.

This rule does not work with a divided clock.

The following code demonstrates the issue.

module tb;
    logic clk = 0;
    always
        #5ns clk = ~clk;

    logic rst_n;
    logic [3:0] d;
    always_ff @(posedge clk, negedge rst_n)
        if (~rst_n)
            d <= '0;
        else
            d <= 4'(d + 4'h1);

    // clock with nonblocking assignment
    logic clk_div_nb;
    always_ff @(posedge clk, negedge rst_n)
        if (~rst_n)
            clk_div_nb <= '0;
        else
            clk_div_nb <= ~clk_div_nb;

    logic [3:0] q_nb_ng;
    always_ff @(posedge clk_div_nb)
        q_nb_ng <= d;   // not work by race condition

    // clock with blocking assignment
    logic clk_div_b;
    always_ff @(posedge clk, negedge rst_n)
        if (~rst_n)
            clk_div_b = '0;
        else
            clk_div_b = ~clk_div_b;

    logic [3:0] q_b_ok;
    always_ff @(posedge clk_div_b)
        q_b_ok  <= d;   // works

    initial begin
        rst_n = 1'b0;
        #10ns
        rst_n = 1'b1;
        // $dumpfile("div_clock.vcd");
        // $dumpvars;
        #100ns
        $stop;
    end
endmodule

q_nb_ng and q_b_ok are D-Flip-flops clocked by divided clocks. They capture the output value of d.

• d is clocked by clk.
• q_nb_ng is clocked by clk_div_nb which is divided by using non-blocking assignment (conforming our current rule).
• q_b_ok is clocked by clk_div_b which is divided by using blocking assignment (as I am proposing this pull-request).

The following is the simulation result of the sample code above.

q_b_ok works as we expect, but q_nb_ok does not. For example at the timing on the yellow line they have to capture value 0b0010 as q_b_ok does.

The following shows how they works on the yellow line.

	previous time slot	1st active region	1st NBA region	2nd active region	2nd NBA region
clk	L	L->H	H	H	H
d	0010	0010	0010->0011	0011	0011
clk_div_nb	L	L	L->H	H	H
q_nb_ng	0001	0001	0001	0001	0001->0011
clk_div_b	L	L->H	H	H	H
q_b_ok	0000	0000	0000->0010	0010	0010

1. clk_div_nb (non-blocking) is asserted at the first NBA region.
2. The state goes back to the second active region and at the 2nd NBA region q_nb_ng captures value (0b0011) of d updated at the first NBA region of this time slot.

This is not the behavior expected. (cf. LRM 2017 Figure 4-1—Event scheduling regions)

---

Plan B: Use #delay for all FFs (when we have a divided clock)

Many engineers are choosing this solution. But we know this degrades simulation performance.

Plan C: Use #delay for FFs clocked by divided clock and capture signals derived from FFs clocked by the original clock.

This may be better performance than plan B, but very easy to cause bugs.

---

I should create a branch after fetching updated upstream. Sorry.

[hirooih]

I tried to send the pull request of the fix on https://github.com/lowRISC/style-guides/pull/44/commits/957948ec27a580aa4f12a371c8dd6291e8b732ac. But something wrong... Sorry.

[rich-ho]

I was pointed to this conversation by a colleague. My thinking about this is along these lines:

The general rules (for flop-based designs) are: D1. State element updates are edge triggered on a clock and assigns must be NB D2. Combinational logic are always blocking. D3. Do not use delays (recommendation for cleanliness and avoidance of simulation-synthesis mismatch)

With these rules in place, I believe you should create all your clocks using blocking assigns. This forces all clock events into the active region and all state updates into the NB region, guaranteed to be after all clocks update.

I believe this is what you're proposing, i.e. to add a style guide statement that all clocks should be generated using blocking assigns. I think that perhaps the wording needs to be clearer and the reason made explicit.

[hirooih]

@rich-ho, thank you for you comment.

I believe this is what you're proposing,

Yes.

• In my proposal always_ff for a clock divider is an exception to non-blocking assign.
• In your proposal always_ff for a clock divider is an exception to "State element".

In either case we need an example code of clock divider for readers.

[hirooih]

I'm sorry I left this PR for a long time.

I've push update, but something wrong.

This branch cannot be rebased due to conflicts Only those with write access to this repository can merge pull requests.

I did "git rebase". Is it the cause?

Sorry for my mess.