state machine and register infering

[Olaf]

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

 

[Paul Uiterlinden]

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)

 

[Andy]

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

 

[Mike Treseler]

[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

 

[Andy]

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;

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,

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

 

[Mike Treseler]

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

 

[Paul Uiterlinden]

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.