state machine and register infering

Discussion in 'VHDL' started by Olaf, May 29, 2007.

  1. Olaf

    Olaf Guest

    Hi,

    I have the following state=output state machine:

    architecture ...
    ....
    signal pattern_match : std_ulogic;
    signal delay_match : std_ulogic;
    signal enable_delay : std_ulogic;

    -- fsm states with one-hot encoding
    subtype state_t is std_ulogic_vector(3 downto 0);

    constant IDLE : state_t := "0001";
    constant ARMED : state_t := "0010";
    constant SHOT_DELAYED : state_t := "0100";
    constant SHOT : state_t := "1000";

    signal state, next_state : state_t;
    begin

    trigger_fsm : process (smpl_clk) is

    variable fsm_enable_delay : std_ulogic;
    variable fsm_match : std_ulogic;

    begin

    fsm_enable_delay := state(1); -- ARMED
    fsm_match := state(3); -- SHOT

    if rising_edge(smpl_clk) then

    state <= next_state; -- avoid latches

    if (reset = RESET_ACTIVE) then
    next_state <= IDLE;
    else

    case state is

    when IDLE => -- State: "0001"
    if (arm = '1') then
    -- go to capture trigger events
    next_state <= ARMED;
    end if;

    when ARMED => -- State: "0010"
    if (pattern_match = '1') and (reg_level =
    global_level) then
    if (or_reduce(reg_timer) = '0') then
    -- no shoting delay timer needed
    next_state <= SHOT;
    else
    -- before we can shot, wait until delay
    next_state <= SHOT_DELAYED;
    end if;
    end if;

    when SHOT_DELAYED => -- State: "0100"
    if (delay_match = '1') then
    next_state <= SHOT;
    end if;

    when SHOT => -- State: "1000"
    -- set the shot and disabled itself
    next_state <= IDLE;

    when others =>
    -- TODO: set status flags; should not be here
    next_state <= IDLE;

    end case;

    end if; -- reset

    end if; -- rising_edge

    enable_delay <= fsm_enable_delay;
    match <= fsm_match;

    end process;

    Unfortunately a register is inferred for enable_delay and match. Try &
    Error doesn't help to remove it X-). E.g.:

    _ _ _ _ _
    clk: _| |_| |_| |_| |_| |
    ------- ------
    state: X 0010 X 1000
    ------- ------
    ______
    match: __| |___

    wanted: ______
    match: | |___

    How to get this? state=output fsm are fast and glitch free following
    Sjoholm and Lindth "VHDL for Designers".

    Thanks
    Olaf
     
    Olaf, May 29, 2007
    #1
    1. Advertising

  2. Olaf wrote:

    > Hi,
    >
    > I have the following state=output state machine:
    >
    > architecture ...
    > ....
    > signal pattern_match : std_ulogic;
    > signal delay_match : std_ulogic;
    > signal enable_delay : std_ulogic;
    >
    > -- fsm states with one-hot encoding
    > subtype state_t is std_ulogic_vector(3 downto 0);
    >
    > constant IDLE : state_t := "0001";
    > constant ARMED : state_t := "0010";
    > constant SHOT_DELAYED : state_t := "0100";
    > constant SHOT : state_t := "1000";
    >
    > signal state, next_state : state_t;
    > begin
    >
    > trigger_fsm : process (smpl_clk) is
    >
    > variable fsm_enable_delay : std_ulogic;
    > variable fsm_match : std_ulogic;
    >
    > begin
    >
    > fsm_enable_delay := state(1); -- ARMED
    > fsm_match := state(3); -- SHOT
    >
    > if rising_edge(smpl_clk) then
    > state <= next_state; -- avoid latches


    Avoid latches? Nonsense! This is a synchronous (clocked) process, so latches
    are not inferred here. That only happens in processes describing
    combinatorial logic.

    [snip]
    > end if; -- rising_edge
    >
    > enable_delay <= fsm_enable_delay;
    > match <= fsm_match;
    >
    > end process;


    I would expect your synthesizer to issue some warnings about the assignments
    outside the "if rising_edge(smpl_clk) then". Everything (besides an
    asynchronous reset) should be inside the "if". What hardware would you
    expect to be generated? Something that changes output on both the rising
    and falling edge of the clock? Because that is what you have written down.

    > Unfortunately a register is inferred for enable_delay and match. Try &
    > Error doesn't help to remove it X-). E.g.:
    >
    > _ _ _ _ _
    > clk: _| |_| |_| |_| |_| |
    > ------- ------
    > state: X 0010 X 1000
    > ------- ------
    > ______
    > match: __| |___
    >
    > wanted: ______
    > match: | |___
    >
    > How to get this?


    You should put the following signal assignments outside the process:

    enable_delay <= state(1);
    match <= state(3)

    --
    Paul Uiterlinden
    www.aimvalley.nl
    e-mail addres: remove the not.
     
    Paul Uiterlinden, May 29, 2007
    #2
    1. Advertising

  3. Olaf

    Andy Guest

    I think you've confused the concept of two process (1 combo, 1
    clocked) state machine descriptions with one process (clocked)
    descriptions.

    With a single process description (my preference, and apparently what
    you are trying to do), you do not need separate state and next_state
    signals. Your state transition logic assigns next_state, but state is
    not updated until the next clock after that. Just use state, and that
    will save you a clock cycle, and perhaps other operational problems.

    Personally, I shy away from explicit one-hot state mappings,
    preferring to let the tool pick the state mapping for me, unless there
    is a performance reason that I need it (i.e. it will not work unless I
    explicitly define it as a certain one hot encoding). That way you can
    avoid the dependency on state encoding for the output. That makes
    maintaining the code later in life easier. Figuring out when match and
    enable are asserted requires knowing the state mapping and the state
    transitions, whereas if they were coded in with the state transition
    logic, it would be more clear and independent of state mapping.

    Andy
     
    Andy, May 29, 2007
    #3
  4. Paul Uiterlinden wrote:

    > [snip]
    >> end if; -- rising_edge
    >>
    >> enable_delay <= fsm_enable_delay;
    >> match <= fsm_match;
    >>
    >> end process;

    >
    > I would expect your synthesizer to issue some warnings about the assignments
    > outside the "if rising_edge(smpl_clk) then". Everything (besides an
    > asynchronous reset) should be inside the "if". What hardware would you
    > expect to be generated? Something that changes output on both the rising
    > and falling edge of the clock? Because that is what you have written down.


    The design has problems, but I don't think this is one of them.
    Since the variable values don't change on the falling edge,
    the double assignments make no logical difference.
    This will synthesize ok as wires from the
    internal register variable to the port/signal. This might
    slow down simulation slightly, but ise, quartus, and leo don't mind.

    -- Mike Treseler
     
    Mike Treseler, May 29, 2007
    #4
  5. Olaf

    Andy Guest

    On May 29, 3:28 pm, Paul Uiterlinden <> wrote:
    > Olaf wrote:
    > > Hi,

    >
    > > I have the following state=output state machine:

    >
    > > architecture ...
    > > ....
    > > signal pattern_match : std_ulogic;
    > > signal delay_match : std_ulogic;
    > > signal enable_delay : std_ulogic;

    >
    > > -- fsm states with one-hot encoding
    > > subtype state_t is std_ulogic_vector(3 downto 0);

    >
    > > constant IDLE : state_t := "0001";
    > > constant ARMED : state_t := "0010";
    > > constant SHOT_DELAYED : state_t := "0100";
    > > constant SHOT : state_t := "1000";

    >
    > > signal state, next_state : state_t;
    > > begin

    >
    > > trigger_fsm : process (smpl_clk) is

    >
    > > variable fsm_enable_delay : std_ulogic;
    > > variable fsm_match : std_ulogic;

    >
    > > begin

    >
    > > fsm_enable_delay := state(1); -- ARMED
    > > fsm_match := state(3); -- SHOT

    >
    > > if rising_edge(smpl_clk) then
    > > state <= next_state; -- avoid latches

    >
    > Avoid latches? Nonsense! This is a synchronous (clocked) process, so latches
    > are not inferred here. That only happens in processes describing
    > combinatorial logic.
    >
    > [snip]
    >
    > > end if; -- rising_edge

    >
    > > enable_delay <= fsm_enable_delay;
    > > match <= fsm_match;

    >
    > > end process;

    >
    > I would expect your synthesizer to issue some warnings about the assignments
    > outside the "if rising_edge(smpl_clk) then". Everything (besides an
    > asynchronous reset) should be inside the "if". What hardware would you
    > expect to be generated? Something that changes output on both the rising
    > and falling edge of the clock? Because that is what you have written down.
    >
    > > Unfortunately a register is inferred for enable_delay and match. Try &
    > > Error doesn't help to remove it X-). E.g.:

    >
    > > _ _ _ _ _
    > > clk: _| |_| |_| |_| |_| |
    > > ------- ------
    > > state: X 0010 X 1000
    > > ------- ------
    > > ______
    > > match: __| |___

    >
    > > wanted: ______
    > > match: | |___

    >
    > > How to get this?

    >
    > You should put the following signal assignments outside the process:
    >
    > enable_delay <= state(1);
    > match <= state(3)
    >
    > --
    > Paul Uiterlindenwww.aimvalley.nl
    > e-mail addres: remove the not.


    Paul,

    Synplify, Precision, and Quartus all accept this coding style (signal
    assignment from expression of variables, after the clock edge clause),
    and implement it as a combinatorial assignment of the expression of
    the registered values of the variables. In this case the expression is
    just the variable value itself, so it becomes a combinatorial "buffer"
    on the registered value of the variable. If the expression involved
    operators, those operators would be combinatorial as well. Such an
    implementation is completely accurate on a clock-cycle basis.

    If the signal assignment is moved to inside the end of the clock edge
    clause, then it becomes a registered assignment of the expression of
    the combinatorial value(s) of the variable(s). Slightly different
    implementation, but with the same clock-cycle behavior, as the RTL
    would suggest. The synthesis tool would identify the signal and
    variable register as the same and combine them. That level of
    optimization is not required if the assignment is made after the clock
    edge clause.

    Andy
     
    Andy, May 29, 2007
    #5
  6. Andy wrote:
    > That level of
    > optimization is not required if the assignment is made after the clock
    > edge clause.


    .... and the RTL view looks cleaner without
    the duplicate registers.

    But it really makes no difference.
    Synthesis removes the duplicate regs
    in any case.

    -- Mike Treseler
     
    Mike Treseler, May 29, 2007
    #6
  7. Mike Treseler wrote:

    > Paul Uiterlinden wrote:
    >> What hardware
    >> would you expect to be generated? Something that changes output on both
    >> the rising and falling edge of the clock? Because that is what you have
    >> written down.

    >
    > The design has problems, but I don't think this is one of them.
    > Since the variable values don't change on the falling edge,
    > the double assignments make no logical difference.
    > This will synthesize ok as wires from the
    > internal register variable to the port/signal. This might
    > slow down simulation slightly, but ise, quartus, and leo don't mind.


    Mike and Andy,

    Thanks for the correction and explanations.

    It clearly shows that I don't use synthesizers a lot.
    Actually: hardly at all. Uhmm, to be honest: never.

    --
    Paul Uiterlinden
    www.aimvalley.nl
    e-mail addres: remove the not.
     
    Paul Uiterlinden, May 29, 2007
    #7
    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. Replies:
    4
    Views:
    584
  2. ashu
    Replies:
    1
    Views:
    496
  3. ashu
    Replies:
    2
    Views:
    648
    mysticlol
    Nov 6, 2006
  4. Grumps
    Replies:
    2
    Views:
    736
    Grumps
    Feb 13, 2008
  5. sniffer
    Replies:
    4
    Views:
    340
    Lie Ryan
    Dec 8, 2008
Loading...

Share This Page