07-30-2020 12:18 PM
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;
07-30-2020 02:54 PM - edited 07-30-2020 02:55 PM
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
07-31-2020 04:31 AM
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
07-31-2020 07:29 AM
> 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.
08-13-2020 06:58 AM - edited 08-21-2020 12:46 PM
...
08-20-2020 04:09 PM
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?
08-20-2020 08:05 PM
LOL. It's not strange. It is beautiful. Adding an extra "_" character wastes disk space:)
To each his own.
08-20-2020 09:09 PM
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.)
08-21-2020 12:47 AM
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).
08-21-2020 09:15 AM - edited 08-21-2020 12:46 PM
...
08-21-2020 11:47 AM
> 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.
08-21-2020 12:47 PM
Well, good luck then.