cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 
Visitor
Visitor
1,081 Views
Registered: ‎07-30-2020

FSM extraction fails on a textbook state machine implementation

Hi,

In Vivado 2020.1, the synthesizer fails to recognize an FSM, which I think is a bug.

This is a minimized version of an issue I encountered while working on a much larger design. I tried to minimize this code further by making small changes to it, but invariably this leads to the FSM recognition to succeed again. For example, getting rid of the 'else' case in the comments or changing 'IN2' in the S3 case to 'IN1' will make FSM recognition work again.

It is completely unclear to me why my particular code inhibits FSM recognition.

Is this a known issue?

Best, Sidney

 

library ieee;
use ieee.std_logic_1164.all;

entity toplevel is
    port (
        CLK      : in  std_logic;
        IN1      : in  std_logic;
        IN2      : in  std_logic;
        OBSERVER : out std_logic
    );
end entity toplevel;

architecture arch of toplevel is

type FsmState is (S1, S2, S3);

signal current_state : FsmState := S1;

begin

    process (CLK) is

    variable next_state: FsmState;

    begin

        if rising_edge(CLK) then

            next_state := current_state;

            case current_state is

                when S1 =>

                    next_state := S2;

                when S2 =>

                    if IN1 = '1' then
                        next_state := S3;
                    elsif IN2 = '1' then
                        next_state := S1;
                    else
                        -- Vivado recognizes an FSM if the assignment below is commented out.
                        -- Leaving it in prevents FSM recognition, even though it is functionally identical.
                        next_state := S2;
                    end if;

                when S3 =>

                    if IN2 = '1' then
                        next_state := S1;
                    else
                        next_state := S2;
                    end if;

            end case;

            current_state <= next_state;

        end if;

    end process;

    OBSERVER <= '1' when current_state = S1 else '0';

end architecture arch;
Tags (3)
0 Kudos
Reply
11 Replies
Scholar
Scholar
1,054 Views
Registered: ‎09-16-2009

Sidney,

For what it's worth, I've found Vivado to be very hit and miss regarding whether or not it recognizes my state machines.  To me, this is mostly a don't care.  FSM extraction is just an optimization.  (some would argue regarding benefits of safe state fault tolerance, but in my opinion this is of very little value).

Your design should work just fine whether or not Vivado recognizes your logic as a "state machine" or not.  I'd argue that any quality of results improvements because Vivado recognized the FSM or not, is mostly in the noise in today's technologies.

My 2 cents.

Regards,

Mark 

0 Kudos
Reply
Xilinx Employee
Xilinx Employee
996 Views
Registered: ‎07-21-2014

Hi @sidneyc 

This looks like a bug to me in FSM detection which can be enhanced. I have filed request for the same.

Nevertheless, as @markcurry has mentioned, the functionality will not be effected with this. the synthesized netlist is equivalent to the RTL with which you can proceed further.

Let us know if you have further concerns o this.

 

-Shreyas

----------------------------------------------------------------------------------------------
Try to search answer for your issue in forums or xilinx user guides before you post a new thread.

Kindly note- Please mark the Answer as "Accept as solution" if information provided solves your query.
Give Kudos (star provided in right) to a post which you think is helpful and reply oriented.
----------------------------------------------------------------------------------------------
0 Kudos
Reply
Visitor
Visitor
973 Views
Registered: ‎07-30-2020

the functionality will not be effected with this.

Well I lose the potential benefit of using a different encoding for my state variable, which may or may not affect the performance of my specific design (not the minimal sample given above, but rather the big design from which it was extracted.)

Thanks for filing a bug report. I'll be eagerly re-checking if this is solved in next releases, but some googling has shown me that FSM-recognition issues have been present in Vivado for some time (years), so I suppose it will take a while.

0 Kudos
Reply
Mentor
Mentor
843 Views
Registered: ‎06-20-2017

0 Kudos
Reply
Adventurer
Adventurer
768 Views
Registered: ‎03-21-2011

In your strange notation I think you meant oObserver, not oBserver.  Not a fan of the prepended i or o without an _, and prefer suffixes so that entire interfaces can prefix the same that include both in and outs.

But the inclusion/non-inclusion of a redundant, vacuous else statement shouldn't be changing compiler behavior.  How they don't reduce down to the same internal representation as the compiler processes them (truth table is the same) boggles my mind.

I'm curious if the original poster had used the form with an async process for next, with curr <= next in a sync process, would the compiler make the same decisions based on the else statement's presence or would behavior be different in that case?

0 Kudos
Reply
Mentor
Mentor
747 Views
Registered: ‎06-20-2017

LOL.  It's not strange.  It is beautiful.   Adding an extra "_" character wastes disk space:) 

 

To each his own.

0 Kudos
Reply
Adventurer
Adventurer
738 Views
Registered: ‎03-21-2011

The nice thing about coding standards is everyone can pick the one they like.

My main complaint is that if you have a whole bunch of related ins and outs:

oSPI_CLK
oSPI_RTC_nEN
iSPI_Dat
oSPI_Dat

is a bit ugly, because even though it's one SPI port, the prefixes are different, compared with

SPI_CLK_o
SPI_RTC_nEN_o
SPI_Dat_i
SPI_Dat_o

(SPI is a bad example because it actually has MISO and MOSI but it's the concept.)

0 Kudos
Reply
Visitor
Visitor
711 Views
Registered: ‎07-30-2020


I'm curious if the original poster had used the form with an async process for next, with curr <= next in a sync process, would the compiler make the same decisions based on the else statement's presence or would behavior be different in that case?

That was the style used when I encountered the problem, yes. (I tend to seperate my design in async/sync processes, loosely following the idiom described here https://www.gaisler.com/doc/vhdl2proc.pdf).

0 Kudos
Reply
Mentor
Mentor
669 Views
Registered: ‎06-20-2017

0 Kudos
Reply
Visitor
Visitor
654 Views
Registered: ‎07-30-2020

> I can see I am not making any friends here

The problem is that in a post that demonstrates a synthesizer bug you somehow feel the need to start lecturing on VHDL style.

If I wanted a lesson in VHDL style, I would have asked for it.

0 Kudos
Reply
Mentor
Mentor
632 Views
Registered: ‎06-20-2017

Well, good luck then.

0 Kudos
Reply