One-hot statemachine design problems

Really a comp.lang.vhdl question, but since you're here... at the beginning of your process (right after the begin), I think you'd want a default assignment for next_state:

next_state stateTransitions : process(current_state, holdoff, read, trig)

The latch warning for next_state is your clue.

Good luck,

Marc

Hi,

I would make a few of suggestions :

• use an enumeration and traditional "case state is when =>" construct. This facilitates a lot of things, if only for the synthesis tool to properly recognize your FSM, but also for HSL simulation, readability, reliability, optimization etc...

• to output the state vector, use a simple decoder (which will be probably eliminated by synthesis). And maybe you don't want the one-hot vector vector to come out, but an binary (more compact) version ?

• Single-process style is easier to write (imo) and not prone to the usual errors in combinational processes. (you're not the first and not the last to get caught) You can even write Moore style in one process if you don't want to move your actions in the transitions.

• Keep in mind that the synthesizer sometimes can guess the "parallel case" and sometime can't. This is likely your case (can't guess). It should have trouble understanding that individual bit comparisons are (probably) mutually exclusive. So I'm afraid it would code the priority you've told him implicitly to implement (last "if" does win). I think this is inefficient synthesis-wise.

On my Website, you'll find many FSM examples, FWIW

I never really liked the "textbook" two-processes style, and I've seen lots of problems in code written this way...

But Text editors, FSM coding and VHDL vs Verilog are almost religious issues :-) ...

Bert

Preben Holm wrote:

I thought the one-hot approach was quite better for performance issues?

Well, you're just right about that, but for simulation purposes this works just fine! I actually only need three bit-outputs!

Well, I tried doing this, but this gave me a lot of trouble so I took the "VHDL made easy" book and did the "cooking book"-version of a one-hot state machine design.

Thanks, i'll take a look!

Maybe, Xilinx recommends this too, in their "templates".

You made the typical mistake --- You didn't clock the inputs. Add one or even two Flip-Flops to every input.

vax, 9000

What is this for? If you are a student and this is part of an assignment I don't want to just give you the answer but that doesn't mean I won't try to help you.

Are there any design requirements that need to be followed or met?

Style and presentation is very important with VHDL. WIth a programming language like 'C' you can get away with creating compound and nested if statements with different structures and with an optimizing compiler end up with the same result.

Early when I was learning VHDL, I noticed that depending on how you structure your code effects the circuit design. In VHDL the compiler tries to implement designs it recognizes ie. state machines.

As, someone else pointed out, the way you went about implementing your state machine isn't the expected way. Try implementng using case structure instead of if control structure. Don't forget the differences between case and if statements - if statements are evaluated sequential and case states are evaluated parallel.

In VHDL as in C, if you have a large chunk of code and are having trouble with it you should break it up into smaller chunks. Use block and process structures to make your code smaller. It should make your code easier to read and make the compilers job easier.

Derek

Quite often it's the case, but using an enumeration does NOT mean at all giving up one-hot encoding !!! Quite the contrary.

For simulation, isn't it nicer to see "Writing" in the waveform viewer rather than "00100" ?

Dave wrote this book a long long time ago, and it was real nice at that time. Today, the synthesizers are very much smarter, so it's better to adopt more modern coding styles which are now well supported by all synthesis tools.

Single process FSM usually means resynchronized Mealy style as I call it, meaning you have to move the state actions up in the arriving transitions. (But there is way to code Moore actions in a single process FSM). That's another way to desoign, but it's often even easier than Moore style. See my Quadrature decoder as an example.

I don't want to sound controversial here, but I wouldn't trust Xilinx as a model of well-written HDL code !

They still deliver this bad code with ISE 6.3.03i (despite my fixing this bad code more than 3 years ago). It's irritating.

You will appreciate the inout (bidirectional buffer) for the LEDs, the absence of resynchronization for the async inputs, the initialization of signals, the absence of reset, the hard tabs in the source file, etc ! (and I won't comment the test bench) Maybe I misread and it was an example of things NOT to do :-)

There are more experts than good code hanging around...

---------cut & paste from ISE 6.3.03i J2C_vhd Example ---------------------- library IEEE; use IEEE.std_logic_1164.all; -- defines std_logic types

entity jc2_top is port ( LEFT : in STD_LOGIC; -- Active-low switch #3 (left) RIGHT : in STD_LOGIC; -- Active-low switch #0 (right) STOP : in STD_LOGIC; -- Active-low switch #2 CLK : in STD_LOGIC; Q : inout STD_LOGIC_VECTOR (3 downto 0) := "0000" -- Active-low LEDs ); end jc2_top;

architecture jc2_top_arch of jc2_top is signal DIR: STD_LOGIC := '0'; -- Left=1, Right=0 signal RUN: STD_LOGIC := '0'; begin

process (CLK) begin if (CLK'event and CLK='1') then -- CLK rising edge

-- DIR register: if (RIGHT='0') then DIR

I am ofcourse a student, but it's quite some time ago that I learned about statemachine design (two years actually). Right now I'm trying to do my bachelor, where I deciden to use VHDL and the Spartan III for my project. I am supposed to build the digital sampling part of a digital storage oscilloscope. But all the theory and implementation which I haven't done a lot of work of is being my trouble. So it's not an assignment, more a tiny part of my own project.

Well, as fast as possible (aiming for 200MHz maximum speed)!

Well, if my litterature is outdated I'm very happy if someone could give me advice for some more updated litterature.

Both the single-process, the xilinx-template, and the one-hot approach I tried to build did the job as being recognized as statemachines (FSM's).

I'm aware of that if-statements are "sequential" (the last if-statement has the "power"), but in hardware they are still quite parallel (even though more if's costs more time in some cases).

Yeah, but I'm not able to break a state machine into smaller pieces, and this is even though quite simple!

How do you like my new design (single-process)

---------------------------------------------- entity holdoffcontroller is Port ( clk : in std_logic; reset : in std_logic; save : in std_logic; trig : in std_logic; read : in std_logic; holdoff : in std_logic; hold : out std_logic := '0'; state : out std_logic_vector(4 downto 0)); end holdoffcontroller;

architecture Behavioral of holdoffcontroller is type states is (stateStart, stateWait, stateTrigger, stateHold, stateRead); begin

STATEMACHINE: block signal current_state : states := stateStart; begin stateRegister : process(clk, reset) begin if reset = '1' then current_state

There, I agree with you entirely ! Making many separate synchronous blocks "talk" to each other is not a good idea (it does waste usually precious clock cycles). I've seen people write a counter outside the one-process FSM which used and controlled it : a mess !

My recommendation = if you do it in one-process : don't split it.

Much better ! Just a few comments

-- I wouldn't initialize the output in the port.

-- nice trick for optional inputs though.

-- I would rather use RTL rather than "behavioral" (cosmetic)

No. It's "bad" enough this initialization does exist in VHDL without you writing it.... The issue of integer range & enums is that they are "automatically" initialized (at simulation) with the leftmost value, so you won't notice they may not be in hardware (synthesis). But since you've coded the async reset (almost) correctly, it's not an issue.

Just don't initialize signals at declaration in RTL code.

btw, you don't need the "current_" prefix anymore, used when you have two different signals, currunt_state (FF otput) and next_state (FF input). That's just cosmetic.

.../...

state '-'); -- play it safe !!!

I would at least make a default assignment on top to (others=>'-') as shown above, in the hope that the synthesis tool will create an optimized one-hot decoder in any (encoding method) case and will never create latches. It's still a VERY good idea to check what synthesis will do (should be just wires here).

Last comment : I would use block / endblock only to restrict the scope of some signals. Since the state information is global (it even goes up in the hierarchy), then the block section (imho) is not useful here.

Hope this help,

Bert Cuzeau

Thanks

Done! But why should I?

RTL meaning?

So the initialization won't be like this in the real hardware - or did I misunderstand you there?

Why will reset show up as enable?