Problem with additions and std_logic

"XSterna" <XSterna@gmail.com> wrote in message
news:3261eb4c-49a4-4cea-8fde-a8baf08f2d51@r66g2000hsg.googlegroups.com...

> Now, the main point I did not really understand is the "all-in-one"
> process one. I understand the synchronous and asynchronous scheme in
> what you wrote. But in my case I don't really catch how to implement
> it.
>

Below is a snippet of your current code with two processes

process(current_state,rw,flag_data_in,data_in,coun ter_write,counter_read,addr)
begin
if rw = '0' then -- write
if flag_data_in = '1' then
next_state <= RAM_write;
else
next_state <= idle;
end if;
elsif rw = '1' then
addr_next <= (others => '0');
next_state <= RAM_read;
else
next_state <= idle;
end if;
end process;

process(CLK,Reset)
begin
if Reset='1' then
current_state<=idle;
counter_write <= (others => '0');
counter_read <= (others => '0');
addr <= (others => '0');
elsif (CLK'event and CLK='1') then
current_state<=next_state;
counter_read <= counter_read_next;
counter_write <= counter_write_next;
addr <= addr_next;
end if;
end process;

Next is functionally the same code with one clocked process

process(CLK,Reset)
begin
if Reset='1' then
current_state<=idle;
counter_write <= (others => '0');
counter_read <= (others => '0');
addr <= (others => '0');
elsif (CLK'event and CLK='1') then
if rw = '0' then -- write
if flag_data_in = '1' then
current_state <= RAM_write; -- No need for 'next_state'
else
current_state <= idle;
end if;
elsif rw = '1' then
addr <= (others => '0'); -- No need for 'addr_next' either
current_state <= RAM_read;
else
current_state <= idle;
end if;
end if;
end process;

Basically, all of your *_next signals are not needed, you update the signals
directly. This is just a snip of your code, but should give you the basic
idea.

Except for possible typing errors on my part, both will synthesize to the
exact same thing. The single clocked process has three advantages over the
two process approach:
- Less typing (therefore less room for error)
- Very small chance of getting the sensitivity list wrong which leads to
synthesized results being functionally different than simulation.
- No chance of creating a latch.

KJ

Once again thank you all for your help

I totally agree the use of only one process is much more simple (and
obviously i prefer it !) ; KJ your three points are right and that's
why I really want to re-code my designs like that. Mike your files are
really interesting for me, even it is a little bit to complicate for
my level, and I "learned" something new with your use of procedure
that I never came across before.

So, I am beginning to understand the one-process thing thanks to your
examples and I tried to modify my most simple entity : the modulo m
counter


First of all, not having a "next signal" imply for me to manage the
first iteration as an exception since it is the only case where I have
a signal a 0 and not a "previous + 1".
I don't really like having to manage exception but maybe there are no
others way to do it.

My main concern is that my design is like "one state" behind. Here is
my code :

entity baud_clk is
generic(
Nbits : integer := 8; -- Number of bits require for the counter
M : integer := 217 -- Counter Modulo
);

Port(
clk : in STD_LOGIC; -- Clock in
rst : in STD_LOGIC; -- reset
baud_tick : out STD_LOGIC -- Output tick at the baud frequency
);
end baud_clk;

architecture arch_baud_clk of baud_clk is
signal counter: unsigned(Nbits-1 downto 0);
signal counter_reg: unsigned(Nbits-1 downto 0);
signal first : boolean;
begin

process(clk,rst)
begin
if (rst = '1') then
counter <= (others => '0'); -- reset of the counter
first <= true;
baud_tick <= '0';
elsif rising_edge(clk) then
if counter = (M-1) then
counter <= (others => '0');
first <= true;
baud_tick <= '1';
else
if first = true then
counter <= (others => '0');
first <= false;
else
counter <= counter_reg + 1;
first <= false;
end if;
baud_tick <='0';
end if;
end if;
counter_reg <= counter;
end process;
end arch_baud_clk;

With this design where I would expect to have baud_tick = '1' when
counter = M-1, I have it at the next iteration... So with a M = 10 for
my simulation, I have baud_tick at 1 after a period of counter = 9 ...

I think I did not catch something

With all that said about combining processes, there are times when you
would want to pull some logic out of the clocked process and describe
it separately. For example, if logic for the condition of an if
statement is shared in several places, you might want to write that as
a concurrent statement or a non-clocked process. I uses non-clocked
processes when a case or if statement is more clear than a conditional
or selected statement, especially when I need to use both which you
can't do in concurrent statements.

process(CLK,Reset)
begin
if Reset='1' then
state <= idle;
elsif (CLK'event and CLK='1') then
case state is
when FOO =>
if ((stuff = 99) and (flag_data_in = '1') or ((stuff = 99) and
(flag_boolean)... then
state <= RAM_write; -- No need for 'next_state'
end if;
when
... other stuff ...
end case;
end if;
end process;

--- or ---

process (the full sensitivity list, many tools will warn you if it is
incomplete)
condition_1 <= FALSE;
case stuff is
when 13 =>
if (flag_data_in = '1') then
condition_1 <= TRUE;
end if;
when 99 =>
if (flag_boolean ...etc...) then
condition_1 <= TRUE;
end if;
...other stuff...
end case;
end process;

process(CLK,Reset)
begin
if Reset='1' then
state <= idle;
elsif (CLK'event and CLK='1') then
case state is
when FOO =>
if (condition_1) then
state <= RAM_write; -- No need for 'next_state'
end if;
when
... other stuff ...
end case;
end if;
end process;


This can make for a much cleaner and more readable clocked process.
Put the details elsewhere if they are very messy or if they are used
somewhere else.

Rick

>
> entity baud_clk is
> * * * * generic(
> * * * * * * * * Nbits : integer := 8; -- Number of bitsrequire for the counter
> * * * * * * * * M : integer := 217 -- Counter Modulo
> * * * * );
>
> * *Port(
> * * * * * * * * clk : in *STD_LOGIC; -- Clock in
> * * * rst : in *STD_LOGIC; -- reset
> * * * baud_tick : out *STD_LOGIC -- Output tick at the baud frequency
> * * * * );
> end baud_clk;
>
> architecture arch_baud_clk of baud_clk is
> * * * * signal counter: unsigned(Nbits-1 downto 0);
> * * * * signal counter_reg: unsigned(Nbits-1 downto 0);
> * * * * signal first : boolean;
> begin
>
> * * * * process(clk,rst)
> * * * * begin
> * * * * * * * * if (rst = '1') then
> * * * * * * * * * * * * counter <= (others =>'0'); -- reset of the counter
> * * * * * * * * * * * * first <= true;
> * * * * * * * * * * * * baud_tick <= '0';
> * * * * * * * * elsif rising_edge(clk) then
> * * * * * * * * * if counter = (M-1) then
> * * * * * * * * * * counter <= (others => '0');
> * * * * * * * * * * first <= true;
> * * * * * * * * * * baud_tick <= '1';
> * * * * * * * * * else
> * * * * * * * * * * if first = true then
> * * * * * * * * * * * counter <= (others => '0');
> * * * * * * * * * * * first <= false;
> * * * * * * * * * * else
> * * * * * * * * * * * counter <= counter_reg + 1;
> * * * * * * * * * * * first <= false;
> * * * * * * * * * * end if;
> * * * * * * * * * * baud_tick <='0';
> * * * * * * * * * *end if;
> * * * * * * * * end if;
> * * * * * * * * counter_reg <= counter;
> * * * * end process;
> end arch_baud_clk;

you dont need counter_reg. You currently have it doing nothing. A
synthesiser would probably see it as just a wire from counter, and you
would get the response you desire. BUT, as the process is only
sensitive you clock and reset, counter reg would actually be 1 clock
cycle behind counter. Also, the first tick would occur 217 clock
cycles after power on, and from then on it would be every 218 clock
cycles, as you have the "first" signal holding the counter at 0 for an
extra clock cycle.

I recommend you remove counter_reg, as counter is implicitly
registered because you have it in a clocked process, so use this
instead:

counter <= counter + 1;

also, I dont quite know why you want the "first" signal, so to
condence the process so that the tick occurs every 217 clock cycles, I
think you'd want this instead:

process(clk,rst)
begin
if (rst = '1') then
counter <= (others => '0'); -- reset of the
counter
baud_tick <= '0';
elsif rising_edge(clk) then
if counter = (M-1) then
--this will cause baud_tick to occur every M clock
cycles.
counter <= (others => '0');
baud_tick <= '1';
else
counter <= counter + 1;
baud_tick <='0';
end if;
end process;

On 5 août, 15:20, rickman <gnu...@gmail.com> wrote:
[...]
> This can make for a much cleaner and more readable clocked process.
> Put the details elsewhere if they are very messy or if they are used
> somewhere else.
>
> Rick

Yes I agree with you. I also feel that only one process could be quite
messy in some cases. I should buy me a notebook to keep tracks of all
those architectures Thank you for the advice !

On 5 août, 15:25, Tricky <Trickyh...@gmail.com> wrote:
> you dont need counter_reg. You currently have it doing nothing. A
> synthesiser would probably see it as just a wire from counter, and you
> would get the response you desire. BUT, as the process is only
> sensitive you clock and reset, counter reg would actually be 1 clock
> cycle behind counter. Also, the first tick would occur 217 clock
> cycles after power on, and from then on it would be every 218 clock
> cycles, as you have the "first" signal holding the counter at 0 for an
> extra clock cycle.
>
> I recommend you remove counter_reg, as counter is implicitly
> registered because you have it in a clocked process, so use this
> instead:
>
> counter <= counter + 1;

Well i just understood that I could do counter <= counter + 1. I don't
really know from where it comes but I usually had a compiling error
doing so. I guess it is with std_logic type I was broadly using for
everything before this topic !
But someone else also told me today that counter <= counter + 1 works
as soon as it is in a process. So now I understand how my code could
be much lighter !

> also, I dont quite know why you want the "first" signal, so to
> condence the process so that the tick occurs every 217 clock cycles, I
> think you'd want this instead:

You are right again it is totally useless.

> process(clk,rst)
> begin
> if (rst = '1') then
> counter <= (others => '0'); -- reset of the
> counter
> baud_tick <= '0';
> elsif rising_edge(clk) then
> if counter = (M-1) then
> --this will cause baud_tick to occur every M clock
> cycles.
> counter <= (others => '0');
> baud_tick <= '1';
> else
> counter <= counter + 1;
> baud_tick <='0';
> end if;
> end process;

This is the disadvantage of being a beginner in something, you take 2h
doing a really messy thing that someone do in 10 minutes (just a
guess in a clear way.

So thank you it is exactly the design I would dream to have. I hope I
will be able to clean my old design in the same way with the one
process architecture. But I really feel I learned what I needed to do
so

Thanks a lot to everyone, you have all been really helpfull !

Xavier

On Aug 5, 10:25 am, Tricky <Trickyh...@gmail.com> wrote:
>
> process(clk,rst)
> begin
> if (rst = '1') then
> counter <= (others => '0'); -- reset of the
> counter
> baud_tick <= '0';
> elsif rising_edge(clk) then
> if counter = (M-1) then
> --this will cause baud_tick to occur every M clock
> cycles.
> counter <= (others => '0');
> baud_tick <= '1';
> else
> counter <= counter + 1;
> baud_tick <='0';
> end if;
> end process;

One suggestion. When implementing counters, it is slightly more
efficient to implement them as loadable down counters.

process(clk,rst)
begin
if (rst = '1') then
counter <= (others => '0'); -- reset of the
counter
baud_tick <= '0';
elsif rising_edge(clk) then
if counter = (0) then
--this will cause baud_tick to occur every M clock
cycles.
counter <= M-1;
baud_tick <= '1';
else
counter <= counter - 1;
baud_tick <='0';
end if;
end process;

This is because in most technologies there is a carry chain built in
that can detect when counter is 0. If you are counting up to (M-1)
the synthesizer has to use LUTs to detect the final state if M is not
a power of 2.

This is a small issue, but if you get used to counting down instead of
up, it will help out in designs where space is tight and potentially
run faster.

Rick

rickman wrote:
>
> One suggestion. When implementing counters, it is slightly more
> efficient to implement them as loadable down counters.
>
>
> This is because in most technologies there is a carry chain built in
> that can detect when counter is 0. If you are counting up to (M-1)
> the synthesizer has to use LUTs to detect the final state if M is not
> a power of 2.
>
> Rick

Good point, but remember up counters are fine too! E.g. divide by 200...

if count = 255 then
count <= 56;
else
count <= count + 1;
end if;

Or something like that.


HTH, Syms.

On Aug 6, 7:36 am, "Symon" <symon_bre...@hotmail.com> wrote:
> rickman wrote:
>
> > One suggestion. When implementing counters, it is slightly more
> > efficient to implement them as loadable down counters.
>
> > This is because in most technologies there is a carry chain built in
> > that can detect when counter is 0. If you are counting up to (M-1)
> > the synthesizer has to use LUTs to detect the final state if M is not
> > a power of 2.
>
> > Rick
>
> Good point, but remember up counters are fine too! E.g. divide by 200...
>
> if count = 255 then
> count <= 56;
> else
> count <= count + 1;
> end if;
>
> Or something like that.
>
> HTH, Syms.

Many synthesis tools will not infer the carry bit from the decrement
for a comparison = 0. They will implement an AND function to test each
bit.

However, if you use integers for counters, it is easy to detect
rollovers with the carry bit.

signal counter : integer range 0 to 2**n-1;
....
if counter - 1 < 0 then -- check the carry bit
counter <= start_val;
do_something;
else
counter <= counter - 1; -- reuse same decrementer
end if;

Note that integer operations are always signed, 32 bit. so the result
of the decrement in the conditional expression can in fact be less
than zero. Not to worry, synthesis will figure out which of the 32
signed bits really gets used, and throw away the rest.

You can also check if counter + 1 > 2**n-1 to get the carry bit from
an incrementer.

This code does not work with unsigned vectors, since the result of
decrementing an unsigned is always unsigned, and can never be larger
than the range defined for the vector (2**n-1).

Andy

Andy wrote:

> You can also check if counter + 1 > 2**n-1 to get the carry bit from
> an incrementer.

> This code does not work with unsigned vectors

In that case, I do something like this:

a_v := a_v + 1;
if a_v(a_v'left) = '1' then -- a carry?
a_v(a_v'left) := '0'; -- clear carry
-- <code enabled by carry goes here>

If the left bit is not otherwise used,
it dissolves into an asynch carry chain bit.

-- Mike Treseler

"Andy" <jonesandy@comcast.net> wrote in message
news:d4ece4bd-1bcd-45b1-bf08-782fd22b5df7@x41g2000hsb.googlegroups.com...
>> rickman wrote:
>> > One suggestion. When implementing counters, it is slightly more
>> > efficient to implement them as loadable down counters.
>> > This is because in most technologies there is a carry chain built in
>> > that can detect when counter is 0. If you are counting up to (M-1)
>> > the synthesizer has to use LUTs to detect the final state if M is not
>> > a power of 2.
>> > Rick
> Many synthesis tools will not infer the carry bit from the decrement
> for a comparison = 0. They will implement an AND function to test each
> bit.
>
> However, if you use integers for counters, it is easy to detect
> rollovers with the carry bit.
> signal counter : integer range 0 to 2**n-1;
> if counter - 1 < 0 then -- check the carry bit
> counter <= start_val;
> do_something;
> else
> counter <= counter - 1; -- reuse same decrementer
> end if;
> Note that integer operations are always signed, 32 bit. so the result
> of the decrement in the conditional expression can in fact be less
> than zero. Not to worry, synthesis will figure out which of the 32
> signed bits really gets used, and throw away the rest.
> Andy

I tried an experiment today with a unit with a 22-bit unsigned counter
(count down). I had initially implemented it as std_logic_vector; and it
turned out initially that I had to stop at one instead of zero. I changed
the logic to a 22-bit unsigned, and changed the load value by one so that
the "stop" point came at cnt_out = 0. Then, I tried it with an integer range
0 to 2*22-1 and the cnt_out -1 test mentioned by Andy.

Using Synplify Pro 8.8 into ISE 9.1, the *unsigned* came out noticeably
smaller (both worked). The *integer* was largest (even bigger than the
initial SLV code). Looking at the RTL view, it seemed like a big and was
still implied in the unsigned case, but I imagine that could be misleading.

I'm going to try a few more and see if the trend continues (at least see if
unsigned works better than SLV, though that difference may have just come
from changing the test from cnt_out = "0000000000000000000001" to cnt_out =
0).

Just one data point...
Marty (a physicist/systems engineer who's rapidly learning VHDL on the fly)