[oc] Verilog coding style for Open Cores-RTL - Case in point SHA1

[Joachim Strömbergson<Joachim.Strombergson@InformAsic.com>]

Aloha!

I've looked through quite a few of the available cores and one thing that 
stands out is quite clearly is the huge difference in coding style between the 
cores. This is obviously natural since everybody and their sister have their 
own ideas, fads and religious beliefs when it comes to writing code. I'm not 
an exception.

But, since there are discussions on this list about how to get bigger adoption 
för Open Cores in the industry [1]. Apart from the never ending licensing 
issues, there are some good ideas about documentation and qualification. All 
very good.

I therefore think that some sort of recommendations in terms of coding style 
is highly appropriate. Ther is a document for this regarding VHDL. But for 
Verilog there is no similar document. If there is an interest for it, I might 
be prepared to "show us the code" and convert/adapt and write preliminary 
design recommendations.

One specific thing in Verilog is the (mis)use of the delay operator "#". There 
are several cores with RTL code that contains this operator. Trust me, this is 
very wrong. Case in point, the SHA1 core by Paul Hartke. In the top level 
entity, you can see:

    // State Elements...
    dffhr #(7) rnd_cnt_reg (.d(rnd_cnt_d), .q(rnd_cnt_q),
                            .clk(clk), .r(reset));
    dffhr #(2) state_reg (.d(next_state), .q(state),
                          .clk(clk), .r(reset));
    dffhr #(512) w_reg (.d(w_d), .q(w_q), .clk(clk), .r(reset));
    dffhr #(160) cv_reg (.d(cv_d), .q(cv_q), .clk(clk), .r(reset));
    dffhr #(160) rnd_reg (.d(rnd_d), .q(rnd_q), .clk(clk), .r(reset));
    dffhr #(160) cv_next_reg (.d(cv_next_d), .q(cv_next_q),
                              .clk(clk), .r(reset));

All synthesis tools will produce warning about "#" not being a synthesisable 
operator. Some, Altera Quartus-II will choke. None of them will generate 
hardware that have been generated due to the "#" operator.

Let me restate this so it's totally clear: If you use "#" in the RTL to get a 
specific behaviour during simulation, then you can be dead certain that the 
real life behaviour of the generated HW will have a different behaviour.

"#" belongs in the testbench and specific simulations models only. Period.

There are quite a few in industry writing code like this too (I've seen it), 
but this is one thing that I see would be a nusiance for people wanting to use 
a core, would lower the trust for OC and therefore be a hindrance to 
adoptions. Fixing things like this, having core that at least somewhat follow 
recommendations about naming, documentation (comments in code and separate 
design documents), clocking, reset etc would, I believe, be a good step forward.


[1] I think one should also ask the question if this is the goal or not. Or 
even interesting, fruitful och good. I think so.
-- 
Med vänlig hälsning, Yours

Joachim Strömbergson - Alltid i harmonisk svängning.
VP, Research & Development
----------------------------------------------------------------------
InformAsic AB / Hugo Grauers gata 5B / SE-411 33 GÖTEBORG / Sweden
Tel: +46 31 68 54 90  Fax: +46 31 68 54 91  Mobile: +46 733 75 97 02
E-mail: joachim.strombergson@informasic.com  Home: www.informasic.com
----------------------------------------------------------------------


--
To unsubscribe from cores mailing list please visit http://www.opencores.org/mailinglists.shtml