Syntax error in VHDL code

Question

I'm trying to implement controller module as a FSM using VHDL, below is the code

entity controller is
 Port ( 
 reset : in STD_LOGIC;
 clk : in STD_LOGIC;
 ring_k_1 : in STD_LOGIC;
 b_n : in STD_LOGIC_vector(3 downto 0);
 start : in STD_LOGIC;
 STOP : out STD_LOGIC;
 LOAD_CMD : out STD_LOGIC;
 ADD_CMD : out STD_LOGIC;
 BYPASS_CMD : out STD_LOGIC);
end controller;
architecture Behavioral of controller is
--declare states
type state_typ is (IDLE, INIT, TEST, ADD, BYPASS);
signal state : state_typ;
begin
process(reset,clk)
 variable i : STD_LOGIC := '0';
 begin
 if reset = '0' then
 state <= IDLE;
 else if clk'event and clk = '1' then
 case state is 
 when IDLE =>
 if START = '1' then
 state <= INIT;
 else
 state <= IDLE;
 end if;
 when INIT =>
 state <= TEST;
 when TEST =>
 if ring_k_1 = '1' then
 state <= IDLE;
 else if ring_k_1 = '0' and b_n(i) = '0' then
 state <= BYPASS;
 i <= i+1;
 else if (ring = '0' and b_n(i) = '1') then
 state <= ADD;
 i <= i+1;
 end if;
 end case; --Syntax error near "case".
 end if; --end for the clock event
 end process; --Syntax error near "process".
 STOP <= '1' when state = IDLE else '0';
 ADD_CMD <= '1' when state = ADD else '0';
 BYPASS_CMD <= '1' when state = BYPASS else '0';
 LOAD_CMD <= '1' when state = INIT else '0';
end Behavioral; --Syntax error near "Behavioral"

I get syntax error when compiled. I have put the description of error as a comment in the code. Can anyone help me as to what should be the correction?

Thanks for the inputs. I did the correction as suggested. Now I'm not getting those errors instead I got some more errors and I corrected all of them but still getting two more errors. Below is the modified code

entity controller is
 Port ( 
reset : in STD_LOGIC;
 clk : in STD_LOGIC;
 ring_k_1 : in STD_LOGIC;
 b_n : in STD_LOGIC_vector(3 downto 0);
 start : in STD_LOGIC;
 STOP : out STD_LOGIC;
 LOAD_CMD : out STD_LOGIC;
 ADD_CMD : out STD_LOGIC;
 BYPASS_CMD : out STD_LOGIC);
end controller;
architecture Behavioral of controller is
--declare states
 type state_typ is (IDLE, INIT, TEST, ADD, BYPASS);
 signal state : state_typ;
 begin
 process(reset,clk)
 variable i : natural := 0;
 begin
 if reset = '0' then
 state <= IDLE;
 STOP <= '1';
 else if clk'event and clk = '1' then
 case state is 
 when IDLE =>
 if START = '1' then
 state <= INIT;
 STOP <= '1';
 else
 state <= IDLE;
 STOP <= '0';
 end if;
 when INIT =>
 LOAD_CMD <= '1';
 state <= TEST;
 when TEST =>
 if ring_k_1 = '1' then
 state <= IDLE;
 elsif ring_k_1 = '0' and b_n(i) = '0' then
 state <= BYPASS;
 BYPASS_CMD <= '1';
 i := i+1;
 elsif (ring_k_1 = '0' and b_n(i) = '1') then
 state <= ADD;
 ADD_CMD <= '1';
 i := i+1;
 end if;
 end case;
 end if; --end for the clock event
end process; --Syntax error near "process".(error 1)
 -- STOP <= '1' when state = IDLE else '0';
 -- ADD_CMD <= '1' when state = ADD else '0';
 -- BYPASS_CMD <= '1' when state = BYPASS else '0';
 -- LOAD_CMD <= '1' when state = INIT else '0';
end Behavioral; --Expecting type void for <behavioral>.(error 2)

I have written error 1 and error2 in the comment to point out the error. Please suggest correction as I'm novice to vhdl coding.

After further modification I get the below error.I scratched my head so much but still no use! Here is the modified code and the error is

ERROR:HDLParsers:164 - "D:/programs_xlinx/BZFAD/controller.vhd" Line 123. parse error, unexpected PROCESS, expecting IF

CODE:

entity controller is
 Port ( reset : in STD_LOGIC;
 clk : in STD_LOGIC;
 ring_k_1 : in STD_LOGIC_vector(3 downto 0);
 b_n : in STD_LOGIC_vector(3 downto 0);
 start : in STD_LOGIC;
 STOP : out STD_LOGIC;
 LOAD_CMD : out STD_LOGIC;
 ADD_CMD : out STD_LOGIC;
 BYPASS_CMD : out STD_LOGIC);
end controller;
architecture Behavioral of controller is
--declare states
type state_typ is (IDLE, INIT, TEST, ADD, BYPASS);
signal state : state_typ;
begin
process(reset,clk)
 --variable i : natural := 0;
 variable i : integer range b_n'RIGHT to b_n'LEFT := b_n'RIGHT; -- 3 downto 0
 begin
 if reset = '1' then
 state <= IDLE;
 STOP <= '1';
 else if clk'event and clk = '1' then
 case state is 
 when IDLE =>
 if START = '1' then
 state <= INIT;
 else
 state <= IDLE;
 STOP <= '1';
 end if;
 when INIT =>
 LOAD_CMD <= '1';
 state <= TEST;
 when TEST =>
 if ring_k_1(3) = '1' then
 state <= IDLE;
 STOP <= '1';
-- 
 elsif ring_k_1(3) = '0' and b_n(i+1) = '0' then
 state <= BYPASS;
 BYPASS_CMD <= '1';
 -- i := i+1;
 if i = b_n'LEFT then 
 i := 0; 
 else 
 i:= i + 1; 
 end if;
 elsif (ring_k_1(3) = '0' and b_n(i+1) = '1') then
 state <= ADD;
 ADD_CMD <= '1';
 if i = b_n'LEFT then 
 i := 0; -- or use i := b_n'RIGHT;
 else 
 i:= i + 1; 
 end if;
 end if;
 when BYPASS =>
 state <= TEST;
 when ADD =>
 state <= TEST;
 end case;
 end if; --end for the clock event
end process;
end Behavioral;

Answer 1

In general an analysis error is associated with a line of your VHDL design description. Comments that are close don't really cut it and the actual error message can be significant.

library ieee;
use ieee.std_logic_1164.all;
entity controller is
 Port ( 
 reset: in std_logic;
 clk: in std_logic;
 ring_k_1: in std_logic;
 b_n: in std_logic_vector(3 downto 0);
 start: in std_logic;
 STOP: out std_logic;
 LOAD_CMD: out std_logic;
 ADD_CMD: out std_logic;
 BYPASS_CMD: out std_logic
 );
end controller;
architecture Behavioral of controller is
 --declare states
 type state_typ is (IDLE, INIT, TEST, ADD, BYPASS);
 signal state : state_typ;
begin
 process(reset,clk)
 variable i : natural := 0; 
 begin
 if reset = '0' then
 state <= IDLE;
 elsif clk'event and clk = '1' then
 case state is 
 when IDLE =>
 if START = '1' then
 state <= INIT;
 else
 state <= IDLE;
 end if;
 when INIT =>
 state <= TEST;
 when TEST =>
 if ring_k_1 = '1' then
 state <= IDLE;
 elsif ring_k_1 = '0' and b_n(i) = '0' then
 state <= BYPASS;
 i := i+1;
 elsif ring_k_1 = '0' and b_n(i) = '1' then
 state <= ADD;
 i := i+1; 
 -- How do you prevent b_n(i) from going outside of 3 downto 0?
 end if;
 when others => -- when ADD, when BYPASS must have all states
 end case; 
 end if;
 end process;
 STOP <= '1' when state = IDLE else '0';
 ADD_CMD <= '1' when state = ADD else '0';
 BYPASS_CMD <= '1' when state = BYPASS else '0';
 LOAD_CMD <= '1' when state = INIT else '0';
end Behavioral;

Besides the corrections you made there is at least one more error with the case statement, in that all the choices must be represented you are missing representation for states ADD and BYPASS:

If the expression is the name of an object whose subtype is locally static,whether a scalar type or an array type, then each value of the subtype must be represented once and only once in the set of choices of the case statement, and no other value is allowed;...
(From the LRM)

I don't know enough about the intent of your design to add branches from states ADD and BYPASS.

Comment out the when others => and ghdl tells us directly that two state enumerations are represented among the choices:

ghdl -a controller.vhdl
controller.vhdl:34:13: no choices for add to bypass
ghdl: compilation error

Error messages aren't standardized in VHDL, ghdl pointed to the line the case statement begins on but did specify the range of values missing from the specified choices.

Notice the simple mechanism used to allow analysis to successfully complete doesn't handle state transitions and likely should.

There's also nothing apparent that will keep b_n(i) within the index range 3 downto 0. Scalar arithmetic operators their conventional mathematical meaning while b_n(i) evaluation will be bounds checked potentially resulting in a run time error should b_n(i) be evaluated and i as an index and fall outside the subtype index range.

Integer arithmetic results can be out of range for use as an index to b_n. You are only using + so only have to limit i to 3 (b_n'LEFT).

If you synthesize the design you'd want to range constrain i to specify the number of bits necessary to implement i (as a counter in this case).

Range constraining i for synthesis implies evaluating for 3 before assigning the new i value to avoid an out of range error. If i = 3, set i to 0 instead, otherwise add 1 to `i1.

In place of i := i+1; use

if i=3 then 
 i := 0; 
else 
 i:= i+1; 
end if;

For synthesis declare the range of i:

variable i : integer range 0 to 3 := 0; 

if b_n'LEFT were the result of a generic or constant declaration and use you could use that instead of 3:

variable i : integer range b_n'RIGHT to b_n'LEFT := b_n'RIGHT; -- 3 downto 0

or

variable i : integer range b_n'RANGE := b_n'RIGHT; -- where b_n'RIGHT = 0

And in place of i := i+1; in your original code use

if i = b_n'LEFT then 
 i := 0; -- or use i := b_n'RIGHT;
else 
 i:= i + 1; 
end if;

Addendum

Your changed code had an else if in place of elsif where the clock was being evaluated:

library ieee;
use ieee.std_logic_1164.all;
entity controller is
 port ( 
 reset: in std_logic;
 clk: in std_logic;
 ring_k_1: in std_logic_vector(3 downto 0);
 b_n: in std_logic_vector(3 downto 0);
 start: in std_logic;
 STOP: out std_logic;
 LOAD_CMD: out std_logic;
 ADD_CMD: out std_logic;
 BYPASS_CMD: out std_logic
 );
end controller;
architecture behavioral of controller is
--declare states
 type state_typ is (IDLE, INIT, TEST, ADD, BYPASS);
 signal state : state_typ;
begin
 process(reset,clk)
 --variable i : natural := 0;
 variable i : integer range b_n'RIGHT to b_n'LEFT := b_n'RIGHT; -- 3 downto 0
 begin
 if reset = '1' then
 state <= IDLE;
 STOP <= '1';
 elsif clk'event and clk = '1' then -- else if
 case state is 
 when IDLE =>
 if START = '1' then
 state <= INIT;
 else
 state <= IDLE;
 STOP <= '1';
 end if;
 when INIT =>
 LOAD_CMD <= '1';
 state <= TEST;
 when TEST =>
 if ring_k_1(3) = '1' then
 state <= IDLE;
 STOP <= '1';
 elsif ring_k_1(3) = '0' and b_n(i+1) = '0' then
 state <= BYPASS;
 BYPASS_CMD <= '1';
 -- i := i+1;
 if i = b_n'LEFT then 
 i := 0; 
 else 
 i:= i + 1; 
 end if;
 elsif (ring_k_1(3) = '0' and b_n(i+1) = '1') then
 state <= ADD;
 ADD_CMD <= '1';
 if i = b_n'LEFT then 
 i := 0; -- or use i := b_n'RIGHT;
 else 
 i:= i + 1; 
 end if;
 end if;
 when BYPASS =>
 state <= TEST;
 when ADD =>
 state <= TEST;
 end case;
 end if; --end for the clock event
 end process;
end behavioral;

The analyzer I was using complained that there should have been if following end instead of process (above end behavioral). It shows among other things the value of readability as well as including the actual error messages:

cont_mod.vhdl:72:9: 'if' is expected instead of 'process' 
ghdl: compilation error 

Note the line number : character position wasn't particularly helpful other than to tell us something else was missing an enclosing end statement. If I'm not mistaken this is the same problem Vladimir Craver pointed out in his answer, which I used as a starting point.

I'd suggest a separate question might be in order should you need help with the simulation results.

Now that your design description for entity controller should analyze perhaps you could ask a separate question should you have trouble with functionality.

I reformatted your second code posting to allow the error to show up a bit easier. Through indentation we don't see any missing level of end if which leaves a syntax error implying the need for another level. else and if in

 else if clk'event and clk = '1' then

implies a separate end if for the else and if.

• user40295: please give me more clarity on what you have said.Maybe some code snippet as to what can be done is good enough to understand.
• user8352: I would have added code for state transitions from ADD and BYPASS to some other states if then intent of your design was clear enough. Integer arithmetic results can be out of range for use as an index to b_n. You are only using "+" so only have to limit to 3 (b_n'LEFT). Range constraining i for synthesis would imply evaluating for 3 before adding and if i=3 set i to 0 instead. In place of i+1' use if i=3 then i := 0; else i:= i+1; end if;. Also declare i range variable i : integer range 0 to 3 := 0;` if b_n'LEFT were the result of a generic or constant use that in place of 3.
• user40295: I did the changes as suggested but still not working!!..
• user8352: "...but still not working!!" isn't terribly specific. Does it analyze? My copy analyzes just fine with the mods. Are you having trouble simulating? As pointed out the design description appears unfinished - there are no choices in your case statement for states ADD and BYPASS, and consequently no way to leave nor actions to take in those states. Without knowing what it's supposed to do, nor a failure mode when simulating someone couldn't possible provide aide with the design, and you requested help with analyzing the controller entity.
• user40295: I have added the states which I had left it before(for ADD and BYPASS)Its showing different error than it was earlier..Please refer the modified version. Sorry for the trouble!.Thanks for the patience!

Answer 2

That's a tricky one that caught me too.

in vhdl the "else if" statement is elsif and NOT else if.

What you are doing actually is something like:

if <condition> then
 statement1;
 statement2;
else --the then after the else is implied
 if <condition> then
 --this actually is an if annidated in the outer one
 statement3;
 statement4;
 end if;
--missing end if for the first if! 

Either you fix your code adding some end if's or you (wise choice) use elsif keyword.

• user40295: Thanks for the input. I modified code and after compilation it gave some more errors I tried solving all that. I could correct some errors and still its giving two more errors .
• user40295: Below is the modified code
• user40295: I have updated my query after making the correction. Please refer the same.
• Vladimir Cravero: you are missing the end case; Please, try to at least read near where the error is reported.
• user40295: I think I have written end case; that is not a problem.