Forum: GHDL anyone confirm this bug in ghdl?

Posted by Steve Franks (Guest)
on 2009-03-26 20:02
(Received via mailing list)
Hi,

I'm seeing something I can't understand - bug?

Look at test3, subtest3.  Note how we pingpong between states 3 & 5.
Somehow that's either a
problem, or I'm missing something obvious....

Best,
Steve


Mutex.vhd
----------------------------------------------------------------------------------
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;
use IEEE.NUMERIC_STD.all;

entity Mutex is port
(
       clk                     : in  std_logic;
       reset           : in  std_logic;

       HatfieldReq             : in  std_logic;
       McCoyReq                : in  std_logic;

       StateOut                        : out std_logic_vector(3 downto 
0);
       NextStateOut            : out std_logic_vector(3 downto 0);

       HatfieldGrant   : out std_logic;
       McCoyGrant      : out std_logic--;
);
end Mutex;

architecture Behavioral of Mutex is

       type states is
       (
               Check_Req,
               Issue_HatfieldGrant, Wait4_HatfieldReqDone,
               Issue_McCoyGrant, Wait4_McCoyReqDone
       );

       signal NextState        : states;
       signal CurrentState: states;

begin

       Mutex_states : process(clk, reset, CurrentState, HatfieldReq, 
McCoyReq)
       begin

               if reset = '1' then

                       CurrentState <= Check_Req;
                       NextState <= Check_Req;
                       HatfieldGrant <= '0';
                       McCoyGrant <= '0';

                       StateOut <= x"0";
                       NextStateOut <= x"1";

               elsif rising_edge(clk) then

                       CurrentState <= NextState;

                       case CurrentState is

                               when Check_Req =>

                                       StateOut <= x"1";

                                       --Just to be sure
                                       HatfieldGrant <= '0';
                                       McCoyGrant <= '0';

                                       if HatfieldReq = '1' then

                                               NextState <= 
Issue_HatfieldGrant;
                                               NextStateOut <= x"2";

                                       else

                                               if McCoyReq = '1' then

                                                       NextState <=
Issue_McCoyGrant;
                                                       NextStateOut <= 
x"4";

                                               else

                                                       NextState <= 
Check_Req;
                                                       NextStateOut <= 
x"1";

                                               end if;

                                       end if;

                               -- Grant ARM access to resource

                               when Issue_HatfieldGrant =>

                                       StateOut <= x"2";
                                       NextStateOut <= x"3";

                                       HatfieldGrant <= '1';
                                       McCoyGrant <= '0';
                                       NextState <= 
Wait4_HatfieldReqDone;

                               when Wait4_HatfieldReqDone =>

                                       StateOut <= x"3";

                                       if HatfieldReq = '1' then

                                               NextState <=
Wait4_HatfieldReqDone;

                                               NextStateOut <= x"3";

                                       else

                                               HatfieldGrant <= '0';
                                               NextState <= Check_Req;

                                               NextStateOut <= x"1";

                                       end if;

                               --  End of ARM access


                               -- Grant PC access to resource

                               when Issue_McCoyGrant =>

                                       StateOut <= x"4";

                                       McCoyGrant <= '1';
                                       HatfieldGrant <= '0';
                                       NextState <= Wait4_McCoyReqDone;

                                       NextStateOut <= x"5";

                               when Wait4_McCoyReqDone =>

                                       StateOut <= x"5";

                                       if McCoyReq = '1' then

                                               NextState <= 
Wait4_McCoyReqDone;

                                               NextStateOut <= x"5";

                                       else

                                               McCoyGrant <= '0';
                                               NextState <= Check_Req;

                                               NextStateOut <= x"1";

                                       end if;

                               --  End of PC access

                               when others =>

                                       StateOut <= x"F";

                                       NextState <= Check_Req;

                       end case;

               end if;

       end process Mutex_states;

end Behavioral;

Mutex_tb.vhd
--------------------------------------------------------------------------------
LIBRARY ieee;
USE ieee.std_logic_1164.ALL;
USE ieee.std_logic_unsigned.all;
USE ieee.numeric_std.ALL;

ENTITY Mutex_tb IS
END Mutex_tb;

ARCHITECTURE behavior OF Mutex_tb IS

   -- Component Declaration for the Unit Under Test (UUT)

   COMPONENT Mutex
   PORT(
        HatfieldReq : IN  std_logic;
        McCoyReq : IN  std_logic;

                StateOut               : out std_logic_vector(3 downto 
0);
                NextStateOut           : out std_logic_vector(3 downto 
0);

        HatfieldGrant : OUT  std_logic;
        McCoyGrant : OUT  std_logic;
        clk : IN  std_logic;
        reset : IN  std_logic
       );
   END COMPONENT;

  --Inputs
  signal HatfieldReq : std_logic := '0';
  signal McCoyReq : std_logic := '0';
  signal clk : std_logic := '0';
  signal reset : std_logic := '0';

       --Outputs
  signal HatfieldGrant : std_logic;
  signal McCoyGrant : std_logic;

  -- Clock period definitions
  constant clk_period : time := 21 ns;

  --Info
  signal State : std_logic_vector(3 downto 0);
  signal NextState : std_logic_vector(3 downto 0);
  signal Testnum : std_logic_vector(3 downto 0);
  signal SubTestnum : std_logic_vector(3 downto 0);

BEGIN

       -- Instantiate the Unit Under Test (UUT)
  uut: Mutex PORT MAP (
         HatfieldReq => HatfieldReq,
         McCoyReq => McCoyReq,

                 StateOut => State,
                 NextStateOut => NextState,

         HatfieldGrant => HatfieldGrant,
         McCoyGrant => McCoyGrant,
         clk => clk,
         reset => reset
       );

  -- Clock process definitions
  clk_process :process
  begin
               clk <= '0';
               wait for clk_period/2;
               clk <= '1';
               wait for clk_period/2;
  end process;


  -- Stimulus process
  stim_proc: process
  begin

  -- hold reset state for 100ms.
       wait for 10 ns;
       wait for clk_period*10;

       Testnum <= x"0";
       SubTestnum <= x"0";

       reset <= '1';
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for clk_period*10;
       reset <= '0';
       wait for 10 us;

       --Slow
       Testnum <= x"1";
       SubTestnum <= x"0";

       SubTestnum <= x"0";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       HatfieldReq <= '1';
       McCoyReq <= '0';
       wait for 999 ns;

       SubTestnum <= x"1";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       HatfieldReq <= '0';
       McCoyReq <= '1';
       wait for 999 ns;

       SubTestnum <= x"2";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       HatfieldReq <= '1';
       McCoyReq <= '1';
       wait for 999 ns;

       --Fast
       Testnum <= x"2";
       SubTestnum <= x"0";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 10 us;

       SubTestnum <= x"0";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for clk_period;

       HatfieldReq <= '1';
       McCoyReq <= '0';
       wait for clk_period;

       SubTestnum <= x"1";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for clk_period;

       HatfieldReq <= '0';
       McCoyReq <= '1';
       wait for clk_period;

       SubTestnum <= x"2";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for clk_period;

       HatfieldReq <= '1';
       McCoyReq <= '1';
       wait for clk_period;

       --Staggered
       Testnum <= x"3";
       SubTestnum <= x"0";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 10 us;

       SubTestnum <= x"0";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       HatfieldReq <= '1';
       wait for clk_period;
       McCoyReq <= '0';
       wait for 999 ns;

       SubTestnum <= x"1";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       HatfieldReq <= '0';
       wait for clk_period;
       McCoyReq <= '1';
       wait for 999 ns;

       SubTestnum <= x"2";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       HatfieldReq <= '1';
       wait for clk_period;
       McCoyReq <= '1';
       wait for 999 ns;

       SubTestnum <= x"3";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       McCoyReq <= '1';
       wait for clk_period;
       HatfieldReq <= '1';
       wait for 999 ns;

       --Switched
       Testnum <= x"4";
       SubTestnum <= x"0";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 10 us;

       SubTestnum <= x"0";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       McCoyReq <= '1';
       wait for clk_period;
       HatfieldReq <= '1';
       wait for clk_period;
       McCoyReq <= '0';
       wait for 999 ns;

       SubTestnum <= x"1";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 999 ns;

       HatfieldReq <= '1';
       wait for clk_period;
       McCoyReq <= '1';
       wait for clk_period;
       HatfieldReq <= '0';
       wait for 999 ns;

       --Nuts
       Testnum <= x"5";
       SubTestnum <= x"0";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for 10 us;

       SubTestnum <= x"0";
       HatfieldReq <= '1';
       McCoyReq <= '1';
       wait for clk_period;

       SubTestnum <= x"1";
       HatfieldReq <= '0';
       McCoyReq <= '1';
       wait for clk_period;

       SubTestnum <= x"2";
       HatfieldReq <= '1';
       McCoyReq <= '0';
       wait for clk_period;

       SubTestnum <= x"3";
       HatfieldReq <= '1';
       McCoyReq <= '1';
       wait for clk_period;

       SubTestnum <= x"4";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for clk_period;

       SubTestnum <= x"5";
       HatfieldReq <= '1';
       McCoyReq <= '0';
       wait for clk_period;

       SubTestnum <= x"6";
       HatfieldReq <= '1';
       McCoyReq <= '1';
       wait for clk_period;

       SubTestnum <= x"7";
       HatfieldReq <= '0';
       McCoyReq <= '1';
       wait for clk_period;

       SubTestnum <= x"8";
       HatfieldReq <= '1';
       McCoyReq <= '1';
       wait for clk_period;

       SubTestnum <= x"9";
       HatfieldReq <= '0';
       McCoyReq <= '0';
       wait for clk_period;


       wait for 999 ns;

       wait;

  end process;

END;
Posted by Wesley J. Landaker (Guest)
on 2009-03-26 20:34
(Received via mailing list)
_______________________________________________
Ghdl-discuss mailing list
Ghdl-discuss@gna.org
https://mail.gna.org/listinfo/ghdl-discuss
Posted by Pascal Giard (Guest)
on 2009-03-26 21:09
(Received via mailing list)
FWIW i get the very same results (but i'm also using 0.27+svn110).

-Pascal

2009/3/26 Wesley J. Landaker <wjl@icecavern.net>:
> how I'd expect from a quick glance at the code. Attached is a snapshot from
>
>



--
Homepage (http://organact.mine.nu)
Debian GNU/Linux (http://www.debian.org)
LACIME: École de technologie supérieure (http://lacime.etsmtl.ca)
Posted by Kendrick Hamilton (Guest)
on 2009-03-26 21:17
(Received via mailing list)
There may be a bug in GHDL, I don't know. However, I suggest a change to
your code.

When writing the process activation list for a flip-flop I usually only
include the clock and asynchronous reset. By having all the other
signals in there you may get latches instead of flip-flops. Try changing

Mutex_states : process(clk, reset, CurrentState, HatfieldReq, McCoyReq)


to

Mutex_states: process(clk, reset)


If your code starts working with this change it means you are
instantiating a latch (bad).

Also, since I use Xilinx FPGAs I usually use synchronous resets, as that
is what the FPGA logic actually has. An asynchronous reset add more 
logic.
Kendrick
Posted by Wesley J. Landaker (Guest)
on 2009-03-26 21:38
(Received via mailing list)
On Thursday 26 March 2009 14:15:26 Kendrick Hamilton wrote:
> When writing the process activation list for a flip-flop I usually only
> include the clock and asynchronous reset. By having all the other
> signals in there you may get latches instead of flip-flops. Try changing
>
> Mutex_states : process(clk, reset, CurrentState, HatfieldReq, McCoyReq)
> to
> Mutex_states: process(clk, reset)

The original code is over specified, but only clk and reset actually 
affect
the process. So having the other signals listed is not good, but doesn't 
do
anything but make it execute slower (and it's more confusing to a human
reader).

> Also, since I use Xilinx FPGAs I usually use synchronous resets, as that
> is what the FPGA logic actually has. An asynchronous reset add more
> logic. Kendrick

(As a totally off-topic note, Xilinx FPGAs actually have both primitive
synchronous and asynchronous reset capability on their FFs, controlled 
by a
per-slice configuration bit. Using one or the other will not affect
resource usage unless you try to use both at the same time.)
Posted by James Rieve (Guest)
on 2009-03-30 01:09
(Received via mailing list)
The problem might be a VHDL coding style / digital logic design issue.
Without spending any time guessing at the intentions of the original
model, I did a quick code/logic clean-up...included below FWIW.

library IEEE;
use IEEE.STD_LOGIC_1164.all;

entity Mutex is
  port (
    clk           : in  std_logic;
    reset         : in  std_logic;
    HatfieldReq   : in  std_logic;
    McCoyReq      : in  std_logic;
    StateOut      : out std_logic_vector(3 downto 0);
    NextStateOut  : out std_logic_vector(3 downto 0);
    HatfieldGrant : out std_logic;
    McCoyGrant    : out std_logic);
end Mutex;

architecture Behavioral of Mutex is

  type states is
    (Check_Req, Issue_HatfieldGrant, Wait4_HatfieldReqDone,
Issue_McCoyGrant, Wait4_McCoyReqDone);

  signal NextState                         : states;
  signal CurrentState                      : states;
  signal StateOut_int, NextStateOut_int    : std_logic_vector(3 downto 
0);
  signal HatfieldGrant_int, McCoyGrant_int : std_logic;

begin

  NextStateLogic : process (CurrentState, HatfieldReq, McCoyReq)
  begin
    case CurrentState is
      when Check_Req =>
        StateOut_int      <= x"1";
        HatfieldGrant_int <= '0';
        McCoyGrant_int    <= '0';
        if (HatfieldReq = '1') then
          NextState        <= Issue_HatfieldGrant;
          NextStateOut_int <= x"2";
        elsif (McCoyReq = '1') then
          NextState        <= Issue_McCoyGrant;
          NextStateOut_int <= x"4";
        else
          NextState        <= Check_Req;
          NextStateOut_int <= x"1";
        end if;

        -- Grant ARM access to resource
      when Issue_HatfieldGrant =>
        StateOut_int      <= x"2";
        NextStateOut_int  <= x"3";
        HatfieldGrant_int <= '1';
        McCoyGrant_int    <= '0';
        NextState         <= Wait4_HatfieldReqDone;

      when Wait4_HatfieldReqDone =>
        StateOut_int   <= x"3";
        McCoyGrant_int <= '0';
        if (HatfieldReq = '1') then
          HatfieldGrant_int <= '1';  -- I am guessing this is the
desired behavior.
          NextState         <= Wait4_HatfieldReqDone;
          NextStateOut_int  <= x"3";
        else
          HatfieldGrant_int <= '0';
          NextState         <= Check_Req;
          NextStateOut_int  <= x"1";
        end if;

        --  End of ARM access
        -- Grant PC access to resource
      when Issue_McCoyGrant =>
        StateOut_int      <= x"4";
        HatfieldGrant_int <= '0';
        McCoyGrant_int    <= '1';
        NextState         <= Wait4_McCoyReqDone;
        NextStateOut_int  <= x"5";

      when Wait4_McCoyReqDone =>
        StateOut_int      <= x"5";
        HatfieldGrant_int <= '0';  -- I am guessing this is the
desired behavior.
        if (McCoyReq = '1') then
          McCoyGrant_int   <= '1';
          NextState        <= Wait4_McCoyReqDone;
          NextStateOut_int <= x"5";
        else
          McCoyGrant_int   <= '0';  -- I am guessing this is the
desired behavior.
          NextState        <= Check_Req;
          NextStateOut_int <= x"1";
        end if;

        --  End of PC access
      when others =>  -- I am guessing this is the desired behavior.
        StateOut_int      <= x"F";
        NextStateOut_int  <= x"1";
        HatfieldGrant_int <= '0';
        McCoyGrant_int    <= '0';
        NextState         <= Check_Req;
    end case;
  end process NextStateLogic;

  Mutex_states : process(clk, reset, NextState, HatfieldGrant_int,
McCoyGrant_int, StateOut_int, NextStateOut_int)
  begin
    if (reset = '1') then
      CurrentState  <= Check_Req;
      HatfieldGrant <= '0';
      McCoyGrant    <= '0';
      StateOut      <= x"0";
      NextStateOut  <= x"1";
    elsif rising_edge(clk) then
      CurrentState  <= NextState;
      HatfieldGrant <= HatfieldGrant_int;
      McCoyGrant    <= McCoyGrant_int;
      StateOut      <= StateOut_int;
      NextStateOut  <= NextStateOut_int;
    end if;
  end process Mutex_states;

end Behavioral;



2009/3/26 Wesley J. Landaker <wjl@icecavern.net>:
This topic is locked and can not be replied to.