linter: Add lint rule for DFF name suffixes

[sifferman]

Summary

Hello! I would like to propose a [dff-name_suffix] lint rule that requires all DFF inputs to be given the suffix _d or _next, all DFF outputs have the suffix _q or _reg, and all DFF input and output pairs have the same base name before the suffix.

Additionally, pipelines can allow _q1, _q2, _q3... or _reg1, _reg2, _reg3...

Test cases and examples

Correct:

logic data_d, data_q;
always_ff @(posedge clk) begin
    if (rst) begin
        data_q <= 0;
    end else begin
        data_q <= data_d;
    end
end

Correct:

logic data_d, data_q1, data_q2, data_q3;
always_ff @(posedge clk) begin
    if (rst) begin
        data_q1 <= 0;
        data_q2 <= 0;
        data_q3 <= 0;
    end else begin
        data_q1 <= data_d;
        data_q2 <= data_q1;
        data_q3 <= data_q2;
    end
end

Incorrect:

logic data_d, data_q;
always_ff @(posedge clk) begin
    assign data_d <= a; // should be driven by assign or always_comb
end
assign data_q = b; // should be driven by always_ff

Incorrect

logic data0_d, data1_q;
always_ff @(posedge clk) begin
    if (rst) begin
        data1_q <= 0;
    end else begin
        data1_q <= data0_d; // can only be driven by data1_d
    end
end

Incorrect:

logic data_d, data_q1, data_q3;
always_ff @(posedge clk) begin
    if (rst) begin
        data_q1 <= 0;
        data_q3 <= 0;
    end else begin
        data_q1 <= data_d;
        data_q3 <= data_q1; // not allowed
    end
end

Additional context

• lowRISC uses _d, _q, and _q<latency>.

[sifferman]

Just noticed that BSG uses _r and _n: https://docs.google.com/document/d/1xA5XUzBtz_D6aSyIBQUwFk_kSUdckrfxa2uzGjMgmCU/edit#heading=h.rhtqn8jwjs44

(I disagree with this style because _n is almost always for active-low signals.)

[IEncinas10]

Hello! A few questions:

1. Are you aware of any other relevant naming conventions? (d vs r, q vs n)
2. Should the optional latency be accepted in any naming convention?

[sifferman]

Hello! Thanks for following up!

1.

I just found the Freescale Verilog Guidelines which use

• _p<latency> for pipeline stages
• _ns for state machine FF inputs (probably not feasible for Verible)
• _ff for FF outputs

It looks like all 4 are fairly commonly used:

Verilog:

• _reg <= 36.0k GitHub results
• _r <= 18.7k GitHub results
• _q <= 3.9k GitHub results
• _ff <= 1.3k GitHub results

SystemVerilog

• _q <= 7.8k GitHub results
• _r <= 6.3k GitHub results
• _reg <= 3.2k GitHub results
• _ff <= 0.9k GitHub results

Also, maybe it would be worth it to have two warnings: one for FF outputs (_reg, _q, _ff), and one for FF inputs (_reg <= _next, _q <= _d, _ff <= _next)? From those searches, it looks like a lot of people choose to always do _reg, but not _next. Plus, Freescale doesn't require a _next.

2.

I think the optional latency would be great for any reg extension! I don't see any issues with that, but I could be missing something

So maybe something like this:

DFF Output	DFF Input
_reg	_next
_r	_n
_ff	_next
_q	_d

Pipeline Output	Pipeline Input
_<DFF_OUT><LATENCY>	_<DFF_IN> or _<DFF_OUT><LATENCY-1>
_p<LATENCY>	_next or _p<LATENCY-1>

[tgorochowik]

Linter rules can actually have some configurability, so we can have a rule that checks a prefix that can be configured (with the ones you suggest set as the defaults).

Would you be willing to implement and contribute such a rule?

[IEncinas10]

I'm a little tight on time currently but I would be able to implement this in some weeks, so if no one picks it up until then I will.