FSM in VHDL which makes me doubt

Question

I'm starting to study VHDL out of personal curiosity, and I'm doing various exercises from a workbook alongside the theory.

However, I’ve come across a solved exercise that raises some doubts for me:

From book: Free Range VHDL of Fabrizio & Bryan Mealy

library IEEE;
use IEEE.std_logic_1164.all;

-- entity
entity my_fsm1 is
    port (
        TOG_EN : in std_logic;
        CLK, CLR : in std_logic;
        Z1 : out std_logic
    );
end my_fsm1;

-- architecture
architecture fsm1 of my_fsm1 is
    type state_type is (ST0, ST1);
    signal PS, NS : state_type;
begin
    sync_proc: process(CLK, CLR)
    begin
        -- take care of the asynchronous input
        if (CLR = '1') then
            PS <= ST0;
        elsif (rising_edge(CLK)) then
            PS <= NS;
        end if;
    end process sync_proc;

    comb_proc: process(PS, TOG_EN)
    begin
        Z1 <= '0'; -- pre-assign output
        case PS is
            when ST0 => -- items regarding state ST0
                Z1 <= '0'; -- Moore output
                if (TOG_EN = '1') then 
                    NS <= ST1;
                else 
                    NS <= ST0;
                end if;
                
            when ST1 => -- items regarding state ST1
                Z1 <= '1'; -- Moore output
                if (TOG_EN = '1') then 
                    NS <= ST0;
                else 
                    NS <= ST1;
                end if;

            when others => -- the catch-all condition
                Z1 <= '0'; -- arbitrary; it should never
                NS <= ST0; -- make it to these two statements
        end case;
    end process comb_proc;

end fsm1;

I have two questions, and despite thinking about them for a while, I can’t understand them on my own.

1. I don’t understand the part where it says

if (CLR = '1') then
    PS <= ST0;
elsif (rising_edge(CLK)) then
    PS <= NS;
end if;

Why is there no catch-all condition? I understand that if I used else instead of elsif (mean)

if (CLR = '1') then
    PS <= ST0;
else
    PS <= NS;
end if;

it would change the logic and is not ok, as I need an activation "IF" the clock rises. However, I read that avoiding a catch-all can lead the synthesized circuit to infer memory and often causes unexpected behaviors. So why not add something like:

if (CLR = '1') then
    PS <= ST0;
elsif (rising_edge(CLK)) then
    PS <= NS;
else
    PS<= PS;
end if;

?

In fact, further down, in another if case, the author uses else to "close" and catch all:

if (TOG_EN = '1') then 
    NS <= ST1;
else 
    NS <= ST0;

2. If I wanted to include a catch-all, how should I do it? Is it correct to write

else PS <= PS;

or is it better to write

else PS <= PS
     NS <= NS;

and could you help me understand why is better PS <= PS or else PS <= PS + NS <= NS because I'm not sure which is better?

I’m not entirely clear on this ambivalence for this finite state machine.

[EDIT] @LordTeddy

Thank you very much for your kind response. What you explained was indeed clear to me; my problem was actually only related to the catch-all, which is something that confuses me a lot.

Where I wrote:

if (TOG_EN = '1') then 
    NS <= ST1;
else 
    NS <= ST0;

I understood that it did not refer to PS, and I was just mentioning it to point out that in this case the if construct had an "else" closure, whereas in the case:

if (CLR = '1') then
    PS <= ST0;
elsif (rising_edge(CLK)) then
    PS <= NS;
end if;

not having a catch-all "else" worried me because I had in mind the information from a course I took, where it was said that "avoiding a catch-all can lead the synthesized circuit to infer memory (generating a latch[]), and can cause unexpected behaviors". []in the sense that if the if and elsif conditions are not verified, a latch is created in the synthesis that causes the value to be the previous one.

Unfortunately, this is a note I took, and I don’t have a bibliographic reference for it, so the intent of my question was to infer what the professor meant by this concept, as I don't have much design experience to date.

To clarify the context, he mentioned that in a machine he had created, he forgot to write a catch-all in one of the states of that FSM, which created a latch that prevented the machine from exiting that state. A catch-all would have resolved that issue and is a good practice to put "else" in if statements. However, he didn't provide an example; he only made this assert, which confused me: because now I’m having trouble understanding when I write a finite state machine whether I should close the statement with an "else" in the management of various inputs checked with "if". This is my big doubt.

For example in the exercise book the author write for another FSM:

when S1 =>
    CTA <= '0';
    if (BUM1 = '0') then
        TOUT <= '0';
        NS <= S1;
    elsif (BUM1 = '1') then
        TOUT <= '1';
        NS <= S2;
    end if;

but I would have put a catch all "else" for what the professor said.

So, this discussion had intrigued me, and every time I see an if, I wonder whether it is useful to use the catch-all "else" or not. That's why I was seeking help from someone knowledgeable on the topic to help me reconstruct the meaning of this phrase.

Answer 1

if (CLR = '1') then
    PS <= ST0;
elsif (rising_edge(CLK)) then
    PS <= NS;
end if;

So this section of the logic is handling the state machine progression. There's only two parts to it.

1. If at any time, CLR becomes high, reset the state to the initial state "ST0"
2. On the rising edge of the CLK, progress to the next state from 'NS'.

There isn't a need for anything else in this. The state will progress each clk cycle, or it will be reset. The reset is asynchronous, in that it doesn't happen on a clk cycle. In terms of defining hardware, this is complete. It's just a D-type flip flop, taking its input from NS, and CLR as an asynchronous reset from CLR. There isn't any other case that needs to be caught.

This instruction

PS <= PS

doesn't do anything. It's loading the value of PS into PS, and I would imagine that this would be optimised out.

if (TOG_EN = '1') then 
    NS <= ST1;
else 
    NS <= ST0;

this section isn't directly relevant to PS. This controls whether the value of NS changes. So it will affect the value of PS at the next clk, but it's part of the logic to implement NS, not PS.

However, I read that avoiding a catch-all can lead the synthesized circuit >to infer memory and often causes unexpected behaviors.

Without a reference to what you've read exactly, it's hard to comment on this. In this particular case, there isn't any catch-all that is relevant so perhaps that's the problem you're having. It does take a bit of time to get your head into defining hardware, as opposed to writing software. If you haven't, it might help to do so some reading on flip-flops, clocking, and digital logic to supplement your VHDL.

[response to edit]

As I've said

if (CLR = '1') then
    PS <= ST0;
elsif (rising_edge(CLK)) then
    PS <= NS;
end if;

this code is complete. There is no need for a "catch-all" as all the behaviour needed is defined. We only have two circumstances under which the value of PS should change, and those are defined. This corresponds to a D-type flip flop and no further statement is necessary. A "catch-all" else statement would just be

else then
--absolutely nothing
endif

and would be optimised away anyway. The term "infer memory" is perhaps related to situations where you're defining asynchronous logic, it might be possible for ill-defined logic to produce memory that wasn't intended, but in this particular case, the value of NS is explicitly stored between clk cycles.

It's also relevant to note, that the above code isn't controlling the progression of the state machine, it's simply loading the next state into the register. There is a catch-all for the actual state machine, the line

where others => -- the catch-all condition

It's not needed, as there's only two states which are both caught, so this final case will not actually be implemented, but in larger state machines you might decide to include something like this to catch any bugs.