Goto page Previous 1, 2
Jonathan Bromley
Guest
Fri Apr 22, 2011 11:03 am
On Thu, 21 Apr 2011 23:12:17 +0100, Chris Hinsley wrote:
Quote:
Agreed. As a matter of interest, could you try it with all
the assignments to pipe_* changed to be <= ? I don't think
that would alter the functionality in this case
But, interesting result if I do put it all to <=, Quartus dosn't
complain, but it (at the RTL level) has the output as an extra stage in
the pipeline !!!
Just a very quick response because it's a nice vacation day
and I'm busy in the garden, but... my bad. When I said
Quote:
I don't think that would alter the functionality
what I really meant was "I didn't think". For pipe[0],
and only for pipe[0], you're reading the variable immediately
after you assigned to it. If you do blocking assignment to
pipe[0] then the immediately subsequent read (in the same
pass through the code, at the same clock edge) will pick
up the newly assigned value - the value that is about to be
saved in pipe[0] by the clock - and so the output is a clone
of pipe[0]. But if you do nonblocking <= assignment to
pipe[0] and then read it, the read picks up the old,
pre-clock value - the value that was stored into pipe[0]
on the previous clock - and so you get an additional stage.
Apologies for misleading you. Interesting, though, that
Quartus has this restriction on assignment to output variables
outwith the clock/reset "if" block.
Quote:
I can just about see why it might do that compared to the
previous code, but it's not obvious.
Well, it was obvious to the synthesis tool and it *should* have
been obvious to me, but I agree that it's tricky to get your
head around it when coming to it for the first time.
Quote:
All good learning fun this !
Yup. I'll try to say a bit more, and respond also to Mark's
very useful post, later today.
cheers
--
Jonathan Bromley
Gabor
Guest
Fri Apr 22, 2011 2:51 pm
Chris Hinsley wrote:
Quote:
Folks, is this a reasonable and correct way to express a pipeline ? I'm
deliberating on things like the use of = or <= in the loop. Advise would
be welcome.
[snip]
for (i = 0; i < DEPTH; i++)
I've always written this as
for (i = 0; i < DEPTH; i = i + 1)
because sometime back (Verilog 95) I found that i++ didn't work. I
certainly use that in C, but not Verilog. Did the autoincrement syntax
get updated with Verilog 2001? Or is this a System Verilog addition?
-- Gabor
Chris Hinsley
Guest
Fri Apr 22, 2011 3:47 pm
On 2011-04-22 15:51:46 +0100, Gabor said:
Quote:
Chris Hinsley wrote:
Folks, is this a reasonable and correct way to express a pipeline ? I'm
deliberating on things like the use of = or <= in the loop. Advise
would be welcome.
[snip]
for (i = 0; i < DEPTH; i++)
I've always written this as
for (i = 0; i < DEPTH; i = i + 1)
because sometime back (Verilog 95) I found that i++ didn't work. I
certainly use that in C, but not Verilog. Did the autoincrement syntax
get updated with Verilog 2001? Or is this a System Verilog addition?
-- Gabor
I just naturally did i++, coming from C++. It means exactly the same thing ?
Chris
Chris Hinsley
Guest
Fri Apr 22, 2011 3:57 pm
Quote:
always @( posedge clk_i )
if( en_i )
shift_regs <= { shift_regs, d_i };
assign q_o = shift_regs[ ( DEPTH - 1 ) * WIDTH +: WIDTH ];
That's basically it. We've got a generate loop to take
case of the special case of DEPTH = 0 (assign q = d).
While I like the simplicity of expressing this in one line, I don't
quite like it.
Are you relying on the compiler to just discard the top part of the
composite value ?
That might be OK, but again, I'm not keen on trusting the compilers to
do this correctly. Is this a garenteed action ?
Chris
Jonathan Bromley
Guest
Fri Apr 22, 2011 5:07 pm
On Fri, 22 Apr 2011 16:47:23 +0100, Chris Hinsley wrote:
Quote:
I just naturally did i++, coming from C++. It means exactly the same thing ?
It was added in SystemVerilog, and has the same meaning as in C.
I believe that some tools still don't support the use of ++/--
in an expression (you know, the usual a[i++] thing). But they
are fine as simple increment operators, equivalent to +=1 (also
supported in SystemVerilog but not in classic Verilog).
The syntax of Verilog bears many superficial resemblances to
C, but it's foolhardy to extrapolate without checking the
consequences. The existence of X values and arbitrary
bit-width expressions makes things distinctly different
in Verilog, and there are other pinch-points too.
--
Jonathan Bromley
Jonathan Bromley
Guest
Fri Apr 22, 2011 5:15 pm
On Fri, 22 Apr 2011 16:57:03 +0100, Chris Hinsley
<chris.hinsley_at_gmail.com> wrote:
Quote:
always @( posedge clk_i )
if( en_i )
shift_regs <= { shift_regs, d_i };
assign q_o = shift_regs[ ( DEPTH - 1 ) * WIDTH +: WIDTH ];
Are you relying on the compiler to just discard the top part of the
composite value ?
Yes, that's completely legitimate and well-defined. Some lint
tools may complain about it as a possible user error, but it's
the mandated behaviour. If you don't like it, widen the target
to the right width:
reg [WIDTH-1:0] discarded;
...
{discarded, shift_regs} <= {shift_regs, d_i};
And then weep at the synthesis warnings saying that
register "discarded" is redundant and has been eliminated!
Quote:
That might be OK, but again, I'm not keen on trusting the
compilers to do this correctly. Is this a garenteed action ?
Yes. Copying a wide expression to a narrow target in Verilog
reliably throws away any surplus MSBs. Of course this is a
two-edged sword, and care is needed. But it's well-defined
and all tools support it correctly.
Widening of narrow expressions when written to a wider target
is a little more complicated, but again it's well-defined
and in simple cases you can just do...
it
reg [3:0] src;
reg [5:0] dst;
...
dst <= src;
and the upper 2 bits of dst will reliably be zero.
--
Jonathan Bromley
Alan Fitch
Guest
Sat Apr 23, 2011 9:08 am
On 22/04/11 18:07, Jonathan Bromley wrote:
Quote:
On Fri, 22 Apr 2011 16:47:23 +0100, Chris Hinsley wrote:
I just naturally did i++, coming from C++. It means exactly the same thing ?
It was added in SystemVerilog, and has the same meaning as in C.
I believe that some tools still don't support the use of ++/--
in an expression (you know, the usual a[i++] thing). But they
are fine as simple increment operators, equivalent to +=1 (also
supported in SystemVerilog but not in classic Verilog).
The syntax of Verilog bears many superficial resemblances to
C, but it's foolhardy to extrapolate without checking the
consequences. The existence of X values and arbitrary
bit-width expressions makes things distinctly different
in Verilog, and there are other pinch-points too.
I'm sure Jonathan knows this, but it caught me out on first encounter -
i++ is equivalent to a blocking assignment, so if you try and write a
counter
always_ff @(posedge clk)
if (reset)
count <= 0;
else
count++;
it will "all end in tears" (or more specifically you'll probably get a
synthesis warning about mixing blocking and non-blocking assignments to
the same variable).
With the for loop, SystemVerilog also allows the loop variable to be local,
for (int i = 0; i< 10; i++)
So if you're committed to SV, you can hide i from the outside world;
which does have a superficial resemblance to C++.
regards
Alan
--
Alan Fitch
Jonathan Bromley
Guest
Sat Apr 23, 2011 12:53 pm
On Sat, 23 Apr 2011 10:08:56 +0100, Alan Fitch wrote:
Quote:
i++ is equivalent to a blocking assignment, so if you
try and write a counter
always_ff @(posedge clk)
if (reset)
count <= 0;
else
count++;
it will "all end in tears" (or more specifically you'll probably get a
synthesis warning about mixing blocking and non-blocking assignments to
the same variable).
Right; all the assignment operators (+=, -= etc) are blocking too.
Which dramatically limits their usefulness for RTL coding!
I recall a frivolous conversation a few years ago, almost
certainly overheard by Alan in the same office, along the
lines of "couldn't you add nonblocking assignment
operators like <+= or <-=" and then deciding that was
going to be a really, really bad idea given that
we already have operators <<= and <<<=.
Quote:
With the for loop, SystemVerilog also allows the loop variable to be local,
for (int i = 0; i< 10; i++)
So if you're committed to SV, you can hide i from the outside world;
which does have a superficial resemblance to C++.
Or even, dare one say it, to VHDL!
--
Jonathan Bromley
Mark Curry
Guest
Mon Apr 25, 2011 4:41 pm
In article <ued3r6hj0o152iavnv4nireip9g2tb16qt_at_4ax.com>,
Jonathan Bromley <spam_at_oxfordbromley.plus.com> wrote:
Quote:
On Fri, 22 Apr 2011 16:57:03 +0100, Chris Hinsley
chris.hinsley_at_gmail.com> wrote:
always @( posedge clk_i )
if( en_i )
shift_regs <= { shift_regs, d_i };
assign q_o = shift_regs[ ( DEPTH - 1 ) * WIDTH +: WIDTH ];
Are you relying on the compiler to just discard the top part of the
composite value ?
Yes, that's completely legitimate and well-defined. Some lint
tools may complain about it as a possible user error, but it's
the mandated behaviour. If you don't like it, widen the target
to the right width:
reg [WIDTH-1:0] discarded;
...
{discarded, shift_regs} <= {shift_regs, d_i};
And then weep at the synthesis warnings saying that
register "discarded" is redundant and has been eliminated!
That might be OK, but again, I'm not keen on trusting the
compilers to do this correctly. Is this a garenteed action ?
Yes. Copying a wide expression to a narrow target in Verilog
reliably throws away any surplus MSBs. Of course this is a
two-edged sword, and care is needed. But it's well-defined
and all tools support it correctly.
Widening of narrow expressions when written to a wider target
is a little more complicated, but again it's well-defined
and in simple cases you can just do...
it
reg [3:0] src;
reg [5:0] dst;
...
dst <= src;
and the upper 2 bits of dst will reliably be zero.
Chris, I tend to be very careful with sizes of things in my designs.
Port sizes must explicity match everywhere, just about all my
constants are sized, being very careful when mixing
algebraic operations with logical operations, etc..
But I rely on these two behaviours in verilog (wide expression to
narrow target, and the reverse) SOO much. As Jonathan indicates -
it's well defined, and the tools will do the correct thing.
Whenever I run some sort of lint tool, I just need
to turn these type of checks completely off.
--Mark
Goto page Previous 1, 2