[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [oc] Verilog coding style for Open Cores-RTL - Case in pointSHA1
Two things here:
First of I want to re-state that I am totally against
Coding/Design Guidelines, as every decent engineer will
have his/her own style and will know what he is doing.
Second, Joachim, shame on you !!!
What Paul has written is perfectly legal and synthesizable
code !
Those are NOT delay statements ! Those are parameters that
are passed the dffhr module. I admit it does look a bit
misleading. What that really does is passes the number in
"#(nnn)" to the modules firs parameter statement.
Looking at Pauls code:
module dffhr (d, r, clk, q);
parameter WIDTH = 1;
....
Now, we see that that parameter is "WIDTH". So Paul is instantiating
the "dffhr" module several times with different width ...
Cheers !
rudi
On Fri, 2003-05-16 at 23:24, Joachim Strömbergson wrote:
> 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.
--
rudi
-------------------------------------------------------
www.asics.ws -- Solutions for your ASIC/FPGA needs ---
---------------- FPGAs * Full Custom ICs * IP Cores ---
* * * FREE IP Cores --> http://www.asics.ws/ <-- * * *
--
To unsubscribe from cores mailing list please visit http://www.opencores.org/mailinglists.shtml