Problem with additions and std_logic

Discussion in 'VHDL' started by XSterna, Aug 3, 2008.

  1. XSterna

    XSterna Guest

    Hi,

    I am having a quite strange problem when I am using the addition with
    std_logic types.

    I am basically using a std_logic_vector containing the address for
    accessing a SRAM. After writing an octect in the RAM I am adding 8 to
    the address for the next word.

    As I did not have the expecting design, I have displayed the content
    of the address vector and I have something ... strange.

    Here are the values I obtain :

    0000 0000
    1100 1000
    1001 0000
    0101 1000
    0010 0000
    1110 1000
    1011 0000
    0111 1000
    0100 0000
    0000 1000
    1101 0000
    etc

    I can see something is quite correct with the 2^0 -> 2^5 bits but only
    for a few time and the 2^6 & 2^7 bits are totally wrong.

    I thought that it could come from the library I used but after
    different tests it does not seem to be the problem. When I try for
    exemple a LED_out <= "0000 0000" + 8; everything is correct ...

    Here is the reduced portion of the concerning code. I have removed
    some signals which does not concern the address mechanism.

    ------------------------------------------------------------

    library IEEE;
    use IEEE.STD_LOGIC_1164.all;
    use IEEE.STD_LOGIC_arith.all;
    use IEEE.STD_LOGIC_UNSIGNED.ALL;


    entity RAM_controller is
    port (CLK, Reset,flag_data_in: in std_logic;
    data_in : in std_logic_vector(7 downto 0);
    data_out : out std_logic_vector(7 downto 0);

    signal_on : in std_logic; -- 0 : mode transfert - 1 : mode signal
    rw : in std_logic; -- 0 : ecriture - 1 : lecture

    sram_io : inout std_logic_vector(15 downto 0);
    sram_a : out std_logic_vector(17 downto 0);
    LED_out : out std_logic_vector(7 downto 0);
    );
    end RAM_controller;

    architecture arch_RAM_controller OF RAM_controller IS
    type state_machine is(idle,RAM_read);
    signal current_state, next_state : state_machine;
    signal I : std_logic_vector(15 downto 0);
    signal counter_write,counter_read : unsigned(7 downto 0);
    signal counter_write_next,counter_read_next : unsigned(7 downto 0);
    signal addr, addr_next : std_logic_vector(17 downto 0);
    begin

    process(CLK,Reset)
    begin
    if Reset='1' then
    current_state<=idle;
    counter_write <= (others => '0');
    counter_read <= (others => '0');
    count <= (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;
    count <= count_next;
    addr <= addr_next;
    end if;
    end process;

    process(current_state,rw,flag_data_in,data_in)
    begin
    counter_read_next <= counter_read;
    counter_write_next <= counter_write;
    addr_next <= addr;

    case current_state is

    when idle=>
    LED_out <= addr;
    if rw = '0' then -- write
    if flag_data_in = '1' then
    next_state <= RAM_write;
    else
    next_state <= idle;
    end if;
    else
    next_state <= idle;
    end if;

    when RAM_write=>--ecriture
    LED_out <= addr;
    sram_a <= addr;
    sram_io <= "00000000" & data_in;
    sram_we <= '0';
    addr_next <= addr + 8;
    counter_write_next <= counter_write + 1;
    next_state <= idle;

    end case;
    end process;
    end arch_RAM_controller;


    ---------------------------------------------
    The flag_data_in is high when a word is transfered by a RS232 link
    (the STOP state of the RS232 controller).

    If anyone has an idea on this problem it would be great because I am
    really clueless on that issue !

    Xavier
     
    XSterna, Aug 3, 2008
    #1
    1. Advertising

  2. XSterna

    rickman Guest

    On Aug 3, 3:03 pm, XSterna <> wrote:
    > Hi,
    >
    > I am having a quite strange problem when I am using the addition with
    > std_logic types.
    >
    > I am basically using a std_logic_vector containing the address for
    > accessing a SRAM. After writing an octect in the RAM I am adding 8 to
    > the address for the next word.
    >
    > As I did not have the expecting design, I have displayed the content
    > of the address vector and I have something ... strange.
    >
    > Here are the values I obtain :
    >
    > 0000 0000
    > 1100 1000
    > 1001 0000
    > 0101 1000
    > 0010 0000
    > 1110 1000
    > 1011 0000
    > 0111 1000
    > 0100 0000
    > 0000 1000
    > 1101 0000
    > etc
    >
    > I can see something is quite correct with the 2^0 -> 2^5 bits but only
    > for a few time and the 2^6 & 2^7 bits are totally wrong.
    >
    > I thought that it could come from the library I used but after
    > different tests it does not seem to be the problem. When I try for
    > exemple a LED_out <= "0000 0000" + 8; everything is correct ...
    >
    > Here is the reduced portion of the concerning code. I have removed
    > some signals which does not concern the address mechanism.
    >
    > ------------------------------------------------------------
    >
    > library IEEE;
    > use IEEE.STD_LOGIC_1164.all;
    > use IEEE.STD_LOGIC_arith.all;
    > use IEEE.STD_LOGIC_UNSIGNED.ALL;
    >
    > entity RAM_controller is
    > port (CLK, Reset,flag_data_in: in std_logic;
    > data_in : in std_logic_vector(7 downto 0);
    > data_out : out std_logic_vector(7 downto 0);
    >
    > signal_on : in std_logic; -- 0 : mode transfert - 1 : mode signal
    > rw : in std_logic; -- 0 : ecriture - 1 : lecture
    >
    > sram_io : inout std_logic_vector(15 downto 0);
    > sram_a : out std_logic_vector(17 downto 0);
    > LED_out : out std_logic_vector(7 downto 0);
    > );
    > end RAM_controller;
    >
    > architecture arch_RAM_controller OF RAM_controller IS
    > type state_machine is(idle,RAM_read);
    > signal current_state, next_state : state_machine;
    > signal I : std_logic_vector(15 downto 0);
    > signal counter_write,counter_read : unsigned(7 downto 0);
    > signal counter_write_next,counter_read_next : unsigned(7 downto 0);
    > signal addr, addr_next : std_logic_vector(17 downto 0);
    > begin
    >
    > process(CLK,Reset)
    > begin
    > if Reset='1' then
    > current_state<=idle;
    > counter_write <= (others => '0');
    > counter_read <= (others => '0');
    > count <= (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;
    > count <= count_next;
    > addr <= addr_next;
    > end if;
    > end process;
    >
    > process(current_state,rw,flag_data_in,data_in)
    > begin
    > counter_read_next <= counter_read;
    > counter_write_next <= counter_write;
    > addr_next <= addr;
    >
    > case current_state is
    >
    > when idle=>
    > LED_out <= addr;
    > if rw = '0' then -- write
    > if flag_data_in = '1' then
    > next_state <= RAM_write;
    > else
    > next_state <= idle;
    > end if;
    > else
    > next_state <= idle;
    > end if;
    >
    > when RAM_write=>--ecriture
    > LED_out <= addr;
    > sram_a <= addr;
    > sram_io <= "00000000" & data_in;
    > sram_we <= '0';
    > addr_next <= addr + 8;
    > counter_write_next <= counter_write + 1;
    > next_state <= idle;
    >
    > end case;
    > end process;
    > end arch_RAM_controller;
    >
    > ---------------------------------------------
    > The flag_data_in is high when a word is transfered by a RS232 link
    > (the STOP state of the RS232 controller).
    >
    > If anyone has an idea on this problem it would be great because I am
    > really clueless on that issue !
    >
    > Xavier


    The code seems overly complicated for what you are trying to do, but
    the only thing I can see that might be a problem is the timing of the
    two input signals, rw and flag_data_in. A write happens when rw is 0
    and flag_data_in is 1. If this combination is asserted for more than
    two clock cycles you will get multiple writes. I say two because the
    first cycle with this asserted causes the state change and the inputs
    are ignored for that cycle. On the third cycle the inputs are checked
    again and if asserted a second write happens. This will continue as
    long as the inputs are asserted.

    Are you running a simulation? Look at the detailed timing of the
    address. Does it change more than once rapidly? Or check the value
    of counter_write. It will increment by 1 for each write. If that
    changes by more than 1, your problem is the enable. Does the write
    counter increment by 25 by any chance?

    A lot of debugging can be done by the Sherlock Holmes method.
    Eliminate whatever is impossible and that which is left must be the
    truth. It looks like it is impossible for the increment by 8 to
    change the address that way in one clock cycle. So it must be
    happening in multiple clock cycles.

    Rick
     
    rickman, Aug 3, 2008
    #2
    1. Advertising

  3. XSterna

    KJ Guest

    "XSterna" <> wrote in message
    news:...
    > Hi,
    >
    > I am having a quite strange problem when I am using the addition with
    > std_logic types.
    >


    Off to a bad start....std_logic_vectors and addition...oh well.

    > I am basically using a std_logic_vector containing the address for
    > accessing a SRAM. After writing an octect in the RAM I am adding 8 to
    > the address for the next word.
    >


    OK.

    > As I did not have the expecting design, I have displayed the content
    > of the address vector and I have something ... strange.
    >


    Displayed on what?
    - Real hardware (possibly hardware issues or timing issues)
    - Simulation (none of that nastiness).

    > Here are the values I obtain :
    >
    > 0000 0000
    > 1100 1000
    > 1001 0000
    > 0101 1000
    > 0010 0000
    > 1110 1000
    > 1011 0000
    > 0111 1000
    > 0100 0000
    > 0000 1000
    > 1101 0000
    > etc
    >
    > I can see something is quite correct with the 2^0 -> 2^5 bits but only
    > for a few time and the 2^6 & 2^7 bits are totally wrong.
    >


    A simulation would likely clear this all up...

    > I thought that it could come from the library I used


    Yes, suspect everything else first...then question your own code and design.

    > but after
    > different tests it does not seem to be the problem.


    Frequently that is the result...library is fine, so it must be the new and
    untested, unvalidated design...go figure.

    > When I try for
    > exemple a LED_out <= "0000 0000" + 8; everything is correct ...
    >



    > Here is the reduced portion of the concerning code. I have removed
    > some signals which does not concern the address mechanism.
    >
    > ------------------------------------------------------------
    >
    > library IEEE;
    > use IEEE.STD_LOGIC_1164.all;
    > use IEEE.STD_LOGIC_arith.all;
    > use IEEE.STD_LOGIC_UNSIGNED.ALL;
    >


    Use ieee.numeric_std instead of std_logic_arith and std_logic_unsigned.
    This isn't the cause of your problem, but numeric_std is an actual standard,
    the others are not.

    >
    > entity RAM_controller is
    > port (CLK, Reset,flag_data_in: in std_logic;
    > data_in : in std_logic_vector(7 downto 0);
    > data_out : out std_logic_vector(7 downto 0);
    >
    > signal_on : in std_logic; -- 0 : mode transfert - 1 : mode signal
    > rw : in std_logic; -- 0 : ecriture - 1 : lecture
    >
    > sram_io : inout std_logic_vector(15 downto 0);
    > sram_a : out std_logic_vector(17 downto 0);
    > LED_out : out std_logic_vector(7 downto 0);
    > );
    > end RAM_controller;
    >


    I wonder where the read and write signals are? Oh well, you're only
    interested in the address so far.

    > architecture arch_RAM_controller OF RAM_controller IS
    > type state_machine is(idle,RAM_read);
    > signal current_state, next_state : state_machine;
    > signal I : std_logic_vector(15 downto 0);
    > signal counter_write,counter_read : unsigned(7 downto 0);
    > signal counter_write_next,counter_read_next : unsigned(7 downto 0);
    > signal addr, addr_next : std_logic_vector(17 downto 0);
    > begin
    >
    > process(CLK,Reset)
    > begin
    > if Reset='1' then
    > current_state<=idle;
    > counter_write <= (others => '0');
    > counter_read <= (others => '0');
    > count <= (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;
    > count <= count_next;
    > addr <= addr_next;
    > end if;
    > end process;
    >


    Synchronous process, so far so good.

    > process(current_state,rw,flag_data_in,data_in)
    > begin
    > counter_read_next <= counter_read;
    > counter_write_next <= counter_write;
    > addr_next <= addr;
    >
    > case current_state is
    >
    > when idle=>
    > LED_out <= addr;
    > if rw = '0' then -- write
    > if flag_data_in = '1' then
    > next_state <= RAM_write;
    > else
    > next_state <= idle;
    > end if;
    > else
    > next_state <= idle;
    > end if;
    >
    > when RAM_write=>--ecriture
    > LED_out <= addr;
    > sram_a <= addr;
    > sram_io <= "00000000" & data_in;
    > sram_we <= '0';
    > addr_next <= addr + 8;
    > counter_write_next <= counter_write + 1;
    > next_state <= idle;
    >
    > end case;
    > end process;


    Hmmm...you seem to be missing quite a few signals from the sensitivity list.
    Quick perusal I see that 'counter_read', 'counter_write', 'addr',
    'RAM_write' seem to be missing. This will create a difference between
    simulation (if you did this) and synthesis. Synthesis should produce a
    warning message to the effect that you have an incomplete sensitivity list
    and list all of the above signals and say that it will be assuming that the
    others should be added....sounds good, but like I said difference between
    sim and actual very bad.

    Options:
    1. Add the signals and re-simulate
    2. Get rid of the two processes and condense it into a single one and
    retest. Splitting into a combinatorial process and a separate synchronous
    process serves no purpose and creates problems such as:
    - The incomplete sensitivity list which creates differences between real
    world and simulation.
    - Possible creation of latches if there is a signal that doesn't get
    assigned for every path through I don't see any instances of this in your
    posted code, but you also stripped it down for the posting so who knows.

    > end arch_RAM_controller;
    >
    >
    > ---------------------------------------------
    > The flag_data_in is high when a word is transfered by a RS232 link
    > (the STOP state of the RS232 controller).
    >
    > If anyone has an idea on this problem it would be great because I am
    > really clueless on that issue !
    >


    Is the input signal 'rw' synchronous to the clk? More generally any input
    to the entity most likely needs to be synchronous to clk, but 'rw' is
    definitely being used in a way that would require it to be synchronous to
    clk.

    Have you simulated this?

    Timing analysis completed successfully?

    KJ
     
    KJ, Aug 3, 2008
    #3
  4. XSterna

    Symon Guest

    Symon, Aug 4, 2008
    #4
  5. XSterna

    XSterna Guest

    I have sent an answer yesterday but it does not seem that it worked !
    In fact rickman was right, I had a bad design with my flag_data_in
    signal which was at the high state for more than one clock cycle. A
    really 'basic' problem that shows actually that my design is not
    really good, but I'm learning :)

    So my actual problem is solved, but I still have others, now with the
    read state ... I think I have to rebuild something cleaner and that is
    why the advices you gave me are good for me :)

    On 4 août, 10:24, "Symon" <> wrote:
    > XSterna wrote:
    >
    > > If anyone has an idea on this problem it would be great because I am
    > > really clueless on that issue !

    >
    > > Xavier

    >
    > http://www.synthworks.com/papers/vhdl_math_tricks_mapld_2003.pdf


    In fact before posting here I read a previous message in this
    newsgroup where this link was given. I read the part about the
    libraries (that's why i talked about it) but when I am using
    ieee.numeric_std I experienced some troubles when I run the
    Synthesize. The actual definition of libraries is the only one which
    seems to work with my code. With the ieee.numeric_std library i have
    this error : "+ can not have such operands in this context." for the

    addr_next <= addr + "1000";
    counter_write_next <= counter_write + 1;

    For a counter signal I read I could use an unsigned type but for the
    address one I can't since it is a 'physical' signal and the unsigned
    type does not work when I synthesize.

    On 3 août, 22:14, "KJ" <> wrote:
    > > I am having a quite strange problem when I am using the addition with
    > > std_logic types.

    >
    > Off to a bad start....std_logic_vectors and addition...oh well.


    But I don't really get why it's not good to use std_logic_vectors. In
    my beginner use, I just saw that I can't use an unsigned as an in or
    out signal. But it's not that I don't want to use them, I just don't
    know why I should and how :)


    > > As I did not have the expecting design, I have displayed the content
    > > of the address vector and I have something ... strange.

    >
    > Displayed on what?
    > - Real hardware (possibly hardware issues or timing issues)
    > - Simulation (none of that nastiness).


    Sorry I forgot an information ! I was displaying it on LEDs to be able
    to visualise it. In fact I usually simulate my designs with modelsim
    but in that case, since I use a SRAM, I can't really simulate it. Part
    of my concern is to know if the SRAM is well "read and written" and a
    Modelsim simulation can not give me that.

    > > I thought that it could come from the library I used

    >
    > Yes, suspect everything else first...then question your own code and design.


    In my case it was because at first I did not have a library which
    understood my '+' operation.

    > Use ieee.numeric_std instead of std_logic_arith and std_logic_unsigned.
    > This isn't the cause of your problem, but numeric_std is an actual standard,
    > the others are not.


    As i wrote previously in this message, I am having troubles with the
    addr signal which is an 'out' one. With numeric_std I can't do a
    addition with std_logic_vector but I need it as an output...

    > > entity RAM_controller is
    > > port (CLK, Reset,flag_data_in: in std_logic;
    > > data_in : in std_logic_vector(7 downto 0);
    > > data_out : out std_logic_vector(7 downto 0);

    >
    > > signal_on : in std_logic; -- 0 : mode transfert - 1 : mode signal
    > > rw : in std_logic; -- 0 : ecriture - 1 : lecture

    >
    > > sram_io : inout std_logic_vector(15 downto 0);
    > > sram_a : out std_logic_vector(17 downto 0);
    > > LED_out : out std_logic_vector(7 downto 0);
    > > );
    > > end RAM_controller;

    >
    > I wonder where the read and write signals are? Oh well, you're only
    > interested in the address so far.


    In fact I wanted to lighten my code to focus on my addr problem. But I
    now think for an experienced designer it would be better to have
    everything to understand ...
    I will give the entire code, but please bear in mind I am beginner :)


    > Hmmm...you seem to be missing quite a few signals from the sensitivity list.
    > Quick perusal I see that 'counter_read', 'counter_write', 'addr',
    > 'RAM_write' seem to be missing. This will create a difference between
    > simulation (if you did this) and synthesis. Synthesis should produce a
    > warning message to the effect that you have an incomplete sensitivity list
    > and list all of the above signals and say that it will be assuming that the
    > others should be added....sounds good, but like I said difference between
    > sim and actual very bad.


    Yes they are missing ... and this is maybe why the rest of my design
    is not working well. I will test it !

    > Options:
    > 1. Add the signals and re-simulate


    When you say simulate do you think about modelsim for example ?
    Because I as told before i don't really understand the good of a
    simulation for the whole design (because for the adrr i agree i would
    have see the problem !) since I can't have the 'real' SRAM signal.

    > 2. Get rid of the two processes and condense it into a single one and
    > retest. Splitting into a combinatorial process and a separate synchronous
    > process serves no purpose and creates problems such as:
    > - The incomplete sensitivity list which creates differences between real
    > world and simulation.
    > - Possible creation of latches if there is a signal that doesn't get
    > assigned for every path through I don't see any instances of this in your
    > posted code, but you also stripped it down for the posting so who knows.
    >


    In fact from the few lectures and books I had about FPGA and VHDL I
    always found two different process with the reset and clk separted
    from the rest. Do you mean that I should do everything on a same
    process with the if rst and if clk at the beginning of it ?


    > Is the input signal 'rw' synchronous to the clk? More generally any input
    > to the entity most likely needs to be synchronous to clk, but 'rw' is
    > definitely being used in a way that would require it to be synchronous to
    > clk.


    rw is a switch so it is not sunchronous to the clk. The Idea of my
    design is to load a file to the SRAM. the rw switch is used to "rw =
    0" write it to the RAM by RS232 ; "rw = 1" read it back by RS232 to
    control to file is well written.

    > Have you simulated this?
    >
    > Timing analysis completed successfully?


    No I don't have any simulation on this. I am visualising the result of
    it with LEDs and with the RS232 communication. It sounds really
    artisanal but one again I did not find any other solution to really be
    able to test the RAM.

    Thanks a lot for your help and your time. If you are curious about my
    design here is the entire code :

    library IEEE;
    use IEEE.STD_LOGIC_1164.all;
    --use IEEE.numeric_std.all;
    use IEEE.STD_LOGIC_arith.all;
    use IEEE.STD_LOGIC_UNSIGNED.ALL;

    entity RAM_controller is
    port (CLK, Reset,flag_data_in: in std_logic;
    data_in : in std_logic_vector(7 downto 0);
    data_out : out std_logic_vector(7 downto 0);

    signal_on : in std_logic; -- 0 : mode transfert - 1 : mode signal
    rw : in std_logic; -- 0 : ecriture - 1 : lecture
    Flag_data_out : out std_logic;


    sram_io : inout std_logic_vector(15 downto 0);
    sram_a : out std_logic_vector(17 downto 0);
    LED_out : out std_logic_vector(7 downto 0);
    sram_ce : out std_logic;
    sram_ub : out std_logic;
    sram_lb : out std_logic;
    sram_we : out std_logic;
    sram_oe : out std_logic
    );
    end RAM_controller;

    architecture arch_RAM_controller OF RAM_controller IS
    type state_machine is(idle,RAM_read,RAM_write);
    signal current_state, next_state : state_machine;
    signal I : std_logic_vector(15 downto 0);
    signal counter_write,counter_read : unsigned(7 downto 0);
    signal counter_write_next,counter_read_next : unsigned(7 downto 0);
    signal addr, addr_next : std_logic_vector(17 downto 0);
    begin

    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;


    process(current_state,rw,flag_data_in,data_in,counter_write,counter_read,addr)
    begin
    counter_read_next <= counter_read;
    counter_write_next <= counter_write;
    addr_next <= addr;

    sram_ce <= '0';
    sram_a <= (others => '0');
    sram_io <= (others => 'Z');
    sram_ce <= '0';
    sram_ub <= '1';
    sram_lb <= '0';
    sram_we <= '1';
    sram_oe <= '1';
    flag_data_out<='0';
    data_out <= (others => '1');
    case current_state is

    when idle=>
    --LED_out <= counter_write(3 downto 0)& counter_read(3 downto 0);
    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;

    when RAM_write=>--ecriture
    --LED_out <= counter_write(3 downto 0)& counter_read(3 downto 0);
    sram_a <= addr;
    sram_io <= "00000000" & data_in;
    sram_we <= '0';
    addr_next <= addr + "1000";
    counter_write_next <= counter_write + 1;

    next_state <= idle;

    when RAM_read=>--lecture
    --LED_out <= counter_write(3 downto 0)& counter_read(3 downto 0);
    if (counter_read < counter_write) then
    sram_a <= addr;
    sram_a <= (others => '0');
    flag_data_out<='1';
    sram_oe <= '0';
    I <= sram_io;
    data_out <= I(7 downto 0);
    addr_next <= addr + 8;
    counter_read_next <= counter_read + 1;
    next_state <= RAM_read;
    else
    next_state <= idle;
    end if;
    end case;
    end process;
    end arch_RAM_controller;
     
    XSterna, Aug 4, 2008
    #5
  6. XSterna

    KJ Guest

    >
    > > XSterna wrote:

    >
    > In fact before posting here I read a previous message in this
    > newsgroup where this link was given. I read the part about the
    > libraries (that's why i talked about it) but when I am using
    > ieee.numeric_std I experienced some troubles when I run the
    > Synthesize. The actual definition of libraries is the only one which
    > seems to work with my code. With the ieee.numeric_std library i have
    > this error : "+ can not have such operands in this context." for the
    >
    > addr_next <= addr + "1000";
    > counter_write_next <= counter_write + 1;
    >


    With addr and addr_next both being std_logic_vectors then you would...
    addr_next <= std_logic_vector(unsigned(addr) + 8);

    The reason for the error is that "1000" is ambiguous. YOU know that
    you mean it to be a binary representation of the number 8, but it is
    also the representation that would be used for -8. When adding/
    subtracting constants, it is always much clearer to simply use
    integers for the constants and not hard code in bit vectors.

    > For a counter signal I read I could use an unsigned type but for the
    > address one I can't since it is a 'physical' signal and the unsigned
    > type does not work when I synthesize.
    >


    No, 'addr' is an internal signal to your architecture; 'sram_a' is the
    entity output so that needs to be a std_logic_vector. Personally, I
    would define 'addr' as

    signal addr: unsigned(sram_a'range);

    I wouldn't have an 'addr_next' signal at all (because of the upcoming
    discussion on two versus one process) but if I did then the
    previously mentioned addition would simplify to

    addr_next <= addr + 8;

    >
    > But I don't really get why it's not good to use std_logic_vectors. In
    > my beginner use, I just saw that I can't use an unsigned as an in or
    > out signal.


    It's not that you can't use unsigned as outputs, it is just that the
    common practice is that the top level signals of a design should all
    be std_logic/std_logic_vector.

    > But it's not that I don't want to use them, I just don't
    > know why I should and how :)
    >


    Why: When you do arithmetic, use unsigned (or signed...sometimes you
    do need to signed arithmetic, like if you wanted to count down to -1
    for some reason).

    How: Simply declare the signal as unsigned (or signed) of the
    appropriate width. Signed/unsigned vectors are really nothing more
    than an interpretation of a collection of bits as having some numeric
    value, whereas std_logic_vector is simply a collection of bits, there
    is no inherent numeric value interpretation implied. When you had
    trouble adding "1000" you mentally had this idea that "1000" is the
    same thing as "8" because you wanted it to have a numeric
    interpretation...but a collection of bits does not *have* to be viewed
    in that manner, in fact since "1000" would just as easily be
    interpreted as "-8" there is a problem.

    When you do have some interface requirement for std_logic_vectors, it
    does not prevent you from creating an internal signal (like 'addr')
    that is of the appropriate type for usage and then when you're all
    done add another concurrent statement that casts the signal to the
    required type. In your case it would be

    sram_a <= std_logic_vector(addr);

    This statement converts the unsigned 'addr' into the std_logic_vector
    'sram_a'. It costs zero hardware, no additional logic gets generated
    to do this since 'unsigned' is simply a particular intepretation of a
    collection of bits, and 'std_logic_vector' is simply a different
    interpretation of the same collection of bits.

    > > Displayed on what?
    > > - Real hardware (possibly hardware issues or timing issues)
    > > - Simulation (none of that nastiness).

    >
    > Sorry I forgot an information ! I was displaying it on LEDs to be able
    > to visualise it. In fact I usually simulate my designs with modelsim
    > but in that case, since I use a SRAM, I can't really simulate it. Part
    > of my concern is to know if the SRAM is well "read and written" and a
    > Modelsim simulation can not give me that.
    >


    You can simulate most anything, including the SRAM. There are likely
    existing models for the SRAM you're using available on the web. Even
    without such a model though, your posting was regarding the address
    outputs to the SRAM you wouldn't need an SRAM model to see this.
    Simulate.

    > > > I thought that it could come from the library I used

    >
    > > Yes, suspect everything else first...then question your own code and design.

    >
    > In my case it was because at first I did not have a library which
    > understood my '+' operation.
    >


    My point was that many blame the tools, libraries, or whatever
    first...when they should be questioning their own code first.

    >
    > > Hmmm...you seem to be missing quite a few signals from the sensitivity list.
    > > Quick perusal I see that 'counter_read', 'counter_write', 'addr',
    > > 'RAM_write' seem to be missing.  This will create a difference between
    > > simulation (if you did this) and synthesis.  Synthesis should produce a
    > > warning message to the effect that you have an incomplete sensitivity list
    > > and list all of the above signals and say that it will be assuming that the
    > > others should be added....sounds good, but like I said difference between
    > > sim and actual very bad.

    >
    > Yes they are missing ... and this is maybe why the rest of my design
    > is not working well. I will test it !
    >


    Good.

    > > Options:
    > > 1. Add the signals and re-simulate

    >
    > When you say simulate do you think about modelsim for example ?


    Use whatever simulator you prefer.

    > Because I as told before i don't really understand the good of a
    > simulation for the whole design (because for the adrr i agree i would
    > have see the problem !) since I can't have the 'real' SRAM signal.


    Sure you can.

    >
    > > 2. Get rid of the two processes and condense it into a single one and
    > > retest.  Splitting into a combinatorial process and a separate synchronous
    > > process serves no purpose and creates problems such as:
    > > - The incomplete sensitivity list which creates differences between real
    > > world and simulation.
    > > - Possible creation of latches if there is a signal that doesn't get
    > > assigned for every path through   I don't see any instances of this in your
    > > posted code, but you also stripped it down for the posting so who knows..

    >
    > In fact from the few lectures and books I had about FPGA and VHDL I
    > always found two different process with the reset and clk separted
    > from the rest.


    The books and lectures are dispensing poor advice. Using a
    combinatorial process and a separate synchronous process as you've
    done leads to the possibility of the above mentioned design errors. A
    good designer avoids methods that can lead to problems or that creates
    extra work, therefore the dispensers of such knowledge would not be
    considered good at design (in my opinion). Such books and lectures
    though are common, so don't feel that you've been cheated, simply
    question them directly if possible about the above two points and ask
    how there method could in any way be considered superior to a method
    that does not have such drawbacks?

    > Do you mean that I should do everything on a same
    > process with the if rst and if clk at the beginning of it ?
    >


    I usually use rst synchronously, but some prefer the async reset. So
    I would use the following template
    process(clk)
    begin
    if rising_edge(clk) then
    if (rst = '1') then
    ...
    else
    ...
    end if;
    end if;
    end process;

    For an async reset I would use the following template

    process(clk,rst)
    begin
    if rising_edge(clk) then
    ...
    end if;

    if (rst = '1') then
    ...
    else
    ...
    end if;
    end process;

    Whether you use rst asynchronously or synchronously, you ALWAYS have
    to use a *synchronized* rst signal (i.e. 'rst' wouldn't come from an
    external input pin of the device, it would be the output of a flip
    flop). The reason is that the trailing edge of 'rst' (i.e. when it
    goes from active to inactive) needs to meet setup time requirements
    relative to the rising edge of 'clk' otherwise, on the first clock
    after 'rst' goes to '0' flip flops in the device could get into states
    that you had not intended.

    > > Is the input signal 'rw' synchronous to the clk?  More generally any input
    > > to the entity most likely needs to be synchronous to clk, but 'rw' is
    > > definitely being used in a way that would require it to be synchronous to
    > > clk.

    >
    > rw is a switch so it is not sunchronous to the clk. The Idea of my
    > design is to load a file to the SRAM. the rw switch is used to "rw =
    > 0" write it to the RAM by RS232 ; "rw = 1" read it back by RS232 to
    > control to file is well written.
    >


    Switches are known to bounce...for long periods of time. One press of
    the switch can likely produce a whole slew of apparent switch
    pushes...if you've never seen this it's fun to watch for the first
    time...a pain then to have add debounce logic later. So...
    1. Synchronize 'rw' to clk before using it for anything.
    2. Be prepared to add debounce logic once you discover what switches
    reeeeeally do.

    > > Have you simulated this?

    >
    > > Timing analysis completed successfully?

    >
    > No I don't have any simulation on this. I am visualising the result of
    > it with LEDs and with the RS232 communication. It sounds really
    > artisanal but one again I did not find any other solution to really be
    > able to test the RAM.


    It's almost always easier to debug in simulation than on real
    hardware. If nothing else, when you get to the real hardware you
    should have more confidence that the design is functionally sound.
    Having said that though, it appears that you also have real world
    issues like switch debounce and asynchronous inputs that will be
    coming up to bite you...and those do not show up in simulation.

    >
    > Thanks a lot for your help and your time.


    You're welcome.

    KJ
     
    KJ, Aug 4, 2008
    #6
  7. XSterna

    Andy Guest

    On Aug 4, 10:12 am, KJ <> wrote:
    > For an async reset I would use the following template
    >
    > process(clk,rst)
    > begin
    > if rising_edge(clk) then
    > ...
    > end if;
    >
    > if (rst = '1') then
    > ...
    > else
    > ...
    > end if;
    > end process;


    KJ has given very good advice, except there should not be an "else" in
    the reset clause (I'm sure it was a typo). I use that template when I
    want to have some storage devices be asynchronously reset, and others
    in the same process not be asynchronously reset.

    Otherwise, I use the more common async reset template:

    process(clk,rst)
    ... -- declarations go here
    begin
    if (rst = '1') then
    ... -- reset assignments go here
    elsif rising_edge(clk) then
    ... -- synchronous assignments go here
    end if;
    end process;

    The only problem is that if you forget to include a register's signal/
    variable in the reset assignments, the synthesis will (should) create
    a feedback mux to ensure that the unreset register maintains its value
    on clock cycles while the async reset is active, which creates timing
    problems. Most synthesis tools will warn you that they are creating
    the feedback mux, which is a hint that you left a reset assignment
    out.

    The following (corrected version of KJ's) template avoids that, but
    also eliminates the warning if you unintentionally left a reset
    assignment out. That's why I don't like using it unless I'm
    intentionally leaving something unreset. Use of this template is a
    reminder for me to double check the reset assignments.

    process(clk,rst)
    ... -- declarations go here
    begin
    if rising_edge(clk) then
    ... -- synchronous assignments go here
    end if;

    if (rst = '1') then
    ... -- reset assignments go here
    end if;
    end process;

    Andy
     
    Andy, Aug 4, 2008
    #7
  8. XSterna

    KJ Guest

    On Aug 4, 1:51 pm, Andy <> wrote:
    > On Aug 4, 10:12 am, KJ <> wrote:
    >
    > KJ has given very good advice, except there should not be an "else" in
    > the reset clause (I'm sure it was a typo).


    Actually it was a copy/paste-o...or more specifically the lack of a
    delete-o following the copy/paste rather than a typo...thanks for the
    catch.

    KJ
     
    KJ, Aug 4, 2008
    #8
  9. XSterna

    XSterna Guest

    Hi,

    Thank you for the explanation about std_logic and unsigned. I
    understand the "difference" and I will do my best to deal with it in a
    better way.

    About the switch, to be honnest I knew it but since my design seemed
    to be working and I did not find my "old" debounce entity, I did not
    used it. But I know I will need one to be sure everything is ok.

    About the simulation, I began to build one for the RAM controller
    because you are right it will be easier to debug and in fact the
    "SRAM" is not really the one which seems to be problematic

    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.

    The separation of the process allows me to manage the next state. I
    don't really see how I will be able to manage the incrementation of a
    signal for example the management of the addr signal. How to increment
    by 8 ? with only one signal i don't see any solution since a <= addr ;
    a<= addr + 8 will give me a <= addr +8 if it is in the same process.

    Well I think I have to work on that because this is a really new
    approach for me :)
     
    XSterna, Aug 4, 2008
    #9
  10. Mike Treseler, Aug 5, 2008
    #10
  11. XSterna

    KJ Guest

    "XSterna" <> wrote in message
    news:...

    > 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,counter_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
     
    KJ, Aug 5, 2008
    #11
  12. XSterna

    XSterna Guest

    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 :)
     
    XSterna, Aug 5, 2008
    #12
  13. XSterna

    rickman Guest

    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
     
    rickman, Aug 5, 2008
    #13
  14. XSterna

    Tricky Guest


    >
    > 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;


    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;
     
    Tricky, Aug 5, 2008
    #14
  15. XSterna

    XSterna Guest

    On 5 août, 15:20, rickman <> 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 <> 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
     
    XSterna, Aug 5, 2008
    #15
  16. XSterna

    rickman Guest

    On Aug 5, 10:25 am, Tricky <> 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, Aug 6, 2008
    #16
  17. XSterna

    Symon Guest

    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.
     
    Symon, Aug 6, 2008
    #17
  18. XSterna

    Andy Guest

    On Aug 6, 7:36 am, "Symon" <> 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, Aug 12, 2008
    #18
  19. 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
     
    Mike Treseler, Aug 12, 2008
    #19
  20. XSterna

    Marty Ryba Guest

    "Andy" <> wrote in message
    news:...
    >> 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)
     
    Marty Ryba, Aug 19, 2008
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. GDan
    Replies:
    11
    Views:
    9,061
    violet
    Jun 29, 2006
  2. Michel Rouzic

    Benchmarking multiplications and additions

    Michel Rouzic, Mar 18, 2006, in forum: C Programming
    Replies:
    10
    Views:
    405
    Michel Rouzic
    Mar 21, 2006
  3. draeath
    Replies:
    10
    Views:
    419
    Dennis Lee Bieber
    Dec 4, 2010
  4. Curt Hibbs
    Replies:
    15
    Views:
    222
    Tom Copeland
    May 17, 2004
  5. David Heinemeier Hansson
    Replies:
    0
    Views:
    148
    David Heinemeier Hansson
    Oct 25, 2004
Loading...

Share This Page