Syntax errors in VHDL - in case statements

Question

I'm very new to VHDL. Here I have a program that calculates GCD of two numbers. I have a bunch of cases and if statements. When I try to simulate, it gives me 6 errors without much description

Errors: 'U:\GCD.dwv' Error 0 line 41 : Syntax error 'U:\GCD.dwv' Error 0 line 43 : Syntax error

The interesting thing is each of them is separated by 2 lines. So it starts with line 33 and goes up to 43 with the same error. It starts on line where "when S3 =>". Here is my code. Thank you!

library IEEE;
use IEEE.std_logic_1164.all;
use IEEE.std_logic_arith.all;

entity GCD is
    port(clk, st : in std_logic; d1, d2 : in std_logic_vector(15 downto 0); dout : out std_logic_vector(15 downto 0); rdy : out std_logic);
end GCD;


architecture behav of GCD is
type state is (S0, S1, S2, S3, S4, S5, S6, S7);
signal new_state : state;
signal eq, It, eq1 : boolean;

begin
        --State Transition Process 
    process is
    variable curr_state : state := S0;
    begin

    if clk ='1' then
        case curr_state is
            when S0 =>
                if st='1' then curr_state := S1;
                end if;
            when S1 =>
                curr_state := S2;
            when S2 =>
                if eq then curr_state := S7;
                else if It then curr_state := S4;
                else if not(eq or It) then curr_state := S3;
                end if;
            when S3 =>
                curr_state := S4;
            when S4 =>
                curr_state := S5;
            when S5 =>
                if eq1 then curr_state := S7;
                else curr_state := S6;
                end if;
            when S6 =>
                curr_state := S1;
            when S7 =>
                if not(st) then curr_state := S0;
                end if;
        end case;
        new_state <= curr_state;
    end if;
    wait on clk;
end process;


-- Asserted Outputs Process:

process is

variable M, N, T, dout_val : std_logic_vector(15 downto 0);
variable rdy_val : std_logic;
variable eq_val, It_val, eq1_val : boolean;
begin

    rdy_val := '0';
    case new_state is
        when S0 =>
            M := d1; N := d2;
        when S1 =>
            eq_val := M=N; It_val := to_integer(M) < to_integer(N);
        when S2 =>
        when S3 =>
            M := T; M := N; N := T;
        when S4 =>
            eq1_val := to_integer(M) = 1;
        when S5 =>
        when S6 =>
            N := N - M; 
        when S7 =>
            rdy_val := '1'; dout_val := M; 
    end case;
    eq <= eq_val;
    It <= It_val;
    eq1 <= eq1_val;
    rdy <= rdy_val;
    dout <= dout_val;
    wait on new_state;
end process;


end behav;

Also be aware that while a simulator probably won't care, synthesis tools may balk at producing flops because you only specify if clk='1' instead of if clk'event and clk='1'. Be careful with variables too. I often see them used by beginners, but they are very rarely needed (especially in fairly simple systems) and easy to make mistakes with. None of the variables you use in this piece of code are needed. Replacing them with signals or direct assignments to the eventual output/signal would clean up the code. — QuantumRipple
– QuantumRipple, Commented Sep 27, 2014 at 1:53
You also have a logic error on the line M := T; M := N; N:= T; for swapping N and M. With signals this is a non-issue - you don't need a temporary value because the right hand side of signal assignments uses the value at the start of process execution (M <= N; N <= M would work as intended with signals). — QuantumRipple
– QuantumRipple, Commented Sep 27, 2014 at 1:56
@QuantumRipple thanks for the help here. For the second one, they aren't signals but ordinary variables so I would need a temporary variable. Right? — NoorUllah
– NoorUllah, Commented Sep 29, 2014 at 3:58
As is, yes the temporary variable is necessary. Using variables at all is unnecessary though. If you made M and N signals and directly assigned the signals where you currently assign the _val variables it would still function the same. Besides being easier to make mistakes with, they also tend to mask the actual complexity of the hardware you are describing. In this case they make your logic appear more complicated than it is, although it's usually the other way around. I try to only use them when they make the description notably easier to read. — QuantumRipple
– QuantumRipple, Commented Sep 29, 2014 at 7:31

score 1 · Accepted Answer · 2014-09-26 00:22:00Z

1

Instead of else if use elsif. There may be a couple more errors lurking in there.

use ieee.std_logic_unsigned.all;  -- because you use std_logic_arith

... State Transition Process:

        when S7 =>
            if st = '0' then -- not (st) then 
                curr_state := S0;
            end if;

... Output Process:

        when S1 =>
            eq_val := M = N; 
            -- It_val := to_integer(M) < to_integer(N);
            It_val := M < N;

        when S4 =>
            -- eq1_val := to_integer(M) = 1;
            eq_val := conv_integer(M) = 1;

VHDL has explicit requirements for separators, the rest is style. Your code requires careful reading because of the lack of consistent style.

Other readers will undoubtedly suggest use use package numeric_std, and if you are using a VHDL -2008 compliant tool, package std_numeric_unsigned instead of Synopsys - non standard use of packages std_logic_arith and std_logic_unsigned. Your attempted use of to_integer is from std_logic_unsigned.

edited Sep 26, 2014 at 0:22

answered Sep 25, 2014 at 23:47

user1155120

Sign up to request clarification or add additional context in comments.

1 Comment

NoorUllah Over a year ago

Thanks David. It worked. Also for the other indications! I cannot use other packages or libraries as this is an assignment and it's my first vhdl program. I'm not sure if we are allowed to.

MtScor · Accepted Answer · 2014-09-26 11:30:34Z

I'd like to give you an example on how I use to write a FSM. I hope It could be useful for your purpose.

LIBRARY ieee;
USE ieee.std_logic_1164.ALL;
USE ieee.numeric_std.ALL;

ENTITY FSM IS
  PORT(
     Clk           : IN  STD_LOGIC;
     nResetLogic   : IN  STD_LOGIC;
     A             : IN  STD_LOGIC;
     B             : IN  STD_LOGIC;
     OUT_A              : OUT STD_LOGIC;
     OUT_B              : OUT STD_LOGIC;
    );
END ENTITY FSM;

ARCHITECTURE RTL OF FSM IS

-- states
TYPE state IS (stateA, stateB, stateC);
signal present_state, next_state : state;

-- Segnals for outputs
signal s_OUT_A              : STD_LOGIC;  
signal s_OUT_B              : STD_LOGIC;  



BEGIN

  -- Sequential section

  PROCESS (nResetLogic, Clk)
  BEGIN

    IF nResetLogic = '0' THEN
      present_state <= stateA;
    ELSIF (RISING_EDGE(Clk)) THEN
      present_state <= next_state;
    END IF;

  END PROCESS;

  -- Comb Section

  PROCESS (A,B,present_state)

  BEGIN
    --defaults
    s_OUT_A <= '0';         
    s_OUT_B <= '0';

    next_state <= present_state;

    CASE present_state IS
        WHEN stateA =>
            -- state definition
            IF (B = '0' AND A = '1') THEN 
                next_state <= stateB;
            ELSIF (B = '1' AND  A = '0') THEN
                next_state <= stateC;
            END IF;

            --output definition
            s_OUT_A <= '0';         
            s_OUT_B <= '1';


        WHEN stateB =>
            -- state definition
            IF (B = '1' AND A = '1') THEN 
                next_state <= stateC;
            ELSIF (B = '1' AND A = '0') THEN 
                next_state <= stateA;

            END IF;

            --output definition
            s_OUT_A <= '1';         
            s_OUT_B <= '1';


        WHEN stateC =>
            -- state definition
            IF (B = '0' AND A = '1') THEN 
                next_state <= statoA;
            ELSIF (B = '1' AND A = '0') THEN
                next_state <= statonB;
            END IF;

            --output definition
            --get defaults        

    END CASE;

  END PROCESS;

  -- Outputs
  OUT_A <= s_OUT_A;         
  OUT_B <= s_OUT_B;  

END ARCHITECTURE RTL;

Thanks @MtScor that is really helpful. Your code looks a lot clean and easier to debug.
You are welcome :) Please vote me to increase my reputation :) I'm pretty new on this website

scary_jeff · Accepted Answer · 2014-09-29 13:04:56Z

There are some good comments on coding style above, but to answer the original question directly, your error is pretty clear when you layout your 'if' statements nicely:

when S2 =>
    if eq then curr_state := S7;
    else if It then curr_state := S4;
    else if not(eq or It) then curr_state := S3;
    end if;

becomes:

when S2 =>
    if eq then
        curr_state := S7;
    else
        if It then
            curr_state := S4;
        else
            if not(eq or It) then
                curr_state := S3;
            end if;

As you can see, there are two missing 'end if' statements. Very often, you will find that an error produced by an FPGA tool, will actually be caused by something on the previous line(s). In this case, the line "when S3 =>" produced the error, because you can't have a "when" by itself in the middle of an "else" block.

Collectives™ on Stack Overflow

Syntax errors in VHDL - in case statements

3 Answers 3

1 Comment

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related