Repeat adder. Works in Sim, not on board (VHDL)

Discussion in 'Programmer's Corner' started by Zeppelin1007, Dec 7, 2012.

  1. Zeppelin1007

    Thread Starter New Member

    Mar 6, 2012
    10
    0
    hey guys. Im pulling out my hair here.

    Im trying to do a repeat adder for class to essentially form a multiplier. Everything simulates fantastic, but as soon as the rubber meats the road and i have to put it on an FPGA board (Xilinx spartan 6) it doesnt work. I added the "after 4ns" on the outputs, just to show that the inputs A and B have been "loaded" and that works fine. Without the time delay, its jumpy and basically inserts random values.

    Once its in, i hit "enable" and klok (manually pulsed clock for testing purposes) and the board does nothing, or, if i play with the if/then statements, just seems to shift random junk everywhere.

    Any ideas guys? Propagation delay issue? as you can probably tell, im more of a C programmer than a VHDL guy, and im sure thats where a chunk of my problem lies.

    Heres my code

    Code ( (Unknown Language)):
    1.  
    2. library ieee;
    3. use ieee.std_logic_1164.all;
    4. use ieee.std_logic_arith.all;
    5. use ieee.std_logic_signed.all;
    6.  
    7.  
    8. entity muladder is
    9. port(
    10.     klok: in std_logic;
    11.     load: in std_logic;
    12.     InputA: in std_logic_vector(7 downto 0):="00000000";
    13.     InputB: in std_logic_vector(3 downto 0):="0000";
    14.     enable: in std_logic;
    15.     Output: out std_logic_Vector(7 downto 0)
    16.     );
    17.  
    18. end muladder;
    19.  
    20. architecture multiplier of muladder is
    21. signal P: std_logic_vector(7 downto 0):="00000000";
    22. signal Countdown: std_logic_vector(3 downto 0):="0000";
    23. signal active: std_logic:='0';
    24. begin
    25.  
    26. process (klok, enable, load, InputA, InputB)
    27.  
    28. begin
    29. if (klok = '1') then
    30.     if (load = '1' and enable = '0') then
    31.     P <= InputA;
    32.     Countdown <= InputB - 1;
    33.     active<= '1';
    34.    
    35.         Output <= InputA after 4ns; --Loads second number to output LEDs
    36.         Output(7 downto 4) <= InputB after 4ns; --loads the first number to the output LEDS
    37.     end if;    
    38. elsif (klok ='1' and enable = '1' ) then
    39.     if( active = '1') then
    40.     P <= InputA + P;
    41.     Countdown <= Countdown - 1;
    42.     Output<= P;
    43.         if Countdown = "0000" then
    44.             active <= '0';
    45.         end if;
    46.     end if;
    47. end if;
    48.  
    49. end process;
    50.  
    51.  
    52. end multiplier;
    53.  
    54.  
    any pointers? thanks in advance
     
  2. WBahn

    Moderator

    Mar 31, 2012
    17,788
    4,808
    Directives like "after 4ns" are for simulation only and are not synthesizable. How could the synthesizer make that happen in an FPGA?

    So anytime your code relies on a delay directive like that (for something that is going to be internal to the FPGA), you have a problem.

    In this case, it looks like, on the same event, you are assigning InputA and InputB to the upper nibble of Output. Bad juju.
     
  3. Zeppelin1007

    Thread Starter New Member

    Mar 6, 2012
    10
    0
    well i rearranged things alittle bit. Turns out its not so much a timing delay issue, its an inferred latch issue.

    I can now get both variables into the "registers" and display them. adding them is a different story though. Once again, simulates fine, but outputs seemingly random jargon

    Code ( (Unknown Language)):
    1.  
    2. library ieee;
    3. use ieee.std_logic_1164.all;
    4. use ieee.std_logic_arith.all;
    5. use ieee.std_logic_signed.all;
    6.  
    7.  
    8. entity muladder is
    9. port(
    10.     klok: in std_logic;
    11.     load: in std_logic:='1';
    12.     InputA: in std_logic_vector(7 downto 0):="00000000";
    13.     InputB: in std_logic_vector(3 downto 0):="0000";
    14.     enable: in std_logic:='0';
    15.     Output: out std_logic_Vector(7 downto 0):="00000000"
    16.     );
    17.  
    18. end muladder;
    19.  
    20. architecture multiplier of muladder is
    21. signal P: std_logic_vector(7 downto 0):="00000000";
    22. signal Countdown: std_logic_vector(3 downto 0):="0000";
    23. signal active: std_logic:='0';
    24. signal lek: std_logic_vector(2 downto 0);
    25. begin
    26.  
    27. process (klok, load, enable, P, InputA, InputB)
    28.  
    29.  
    30.  
    31.  
    32. begin    
    33.  
    34. if (klok = '1' and load = '1' and enable = '0') then
    35.         P(3 downto 0) <= InputA(3 downto 0);
    36.         Countdown <= InputB;
    37.         Output(3 downto 0) <= InputA(3 downto 0); --Loads second number to output LEDs
    38.         Output(7 downto 4) <= InputB; --loads the first number to the output LEDS
    39.        
    40. elsif(klok = '1' and load = '0' and enable = '1') then
    41.         P <= InputA + P;
    42.         Countdown <= Countdown - 1;
    43.         Output <= P;
    44.    
    45. else
    46.         output<=P;
    47.        
    48.        
    49. end if;
    50.  
    51.  
    52. end process;
    53.  
    54.  
    55. end multiplier;
    56.  
    57.  
    from what i can gather, i need to cover all bases and all possible input configurations on the if/thens. Is there a better approach?

    Basically the whole premise of this is, input 2 numbers A and B using toggle switches. Press load, load "displays" them on the LED's as output to sort of confirm theyve been loaded. Then "enable" allows the adding to take place, and everytime a number is added to itself it displays on the LED's

    thanks in advance guys
     
  4. WBahn

    Moderator

    Mar 31, 2012
    17,788
    4,808
    Nope. And how can there be? If you don't specify what you want to have happen for all possible input conditions, what is the synthesizer supposed to do? It's not a mind reader. The best it can do is assume that what you mean is that the output shouldn't change, which means it has to infer a latch.

    This is somewhat contradictory. You first talk about two numbers being added and then talk about a number being added to itself.
     
    Last edited: Dec 9, 2012
  5. Zeppelin1007

    Thread Starter New Member

    Mar 6, 2012
    10
    0
    Sorry Wbahn, ive been staring at this so long its beginning to lose meaning.

    I sort of solved my output timing issue. You were right, the after delay is only simulation. I can also get output on the board....though i have a phantom bit showing up. My output on the left side (top 4 bits, 7 downto 4) randomly show up after calculations as 0010, 0011, 0110, 1001..like its counting by 3's, and the right side actually adds to itself like it should, and stops once its complete.

    Code (Text):
    1.  
    2.  
    3. library ieee;
    4. use ieee.std_logic_1164.all;
    5. use ieee.std_logic_arith.all;
    6. use ieee.std_logic_signed.all;
    7.  
    8.  
    9. entity muladder is
    10. port(
    11.     klok: in std_logic;
    12.     load: in std_logic;
    13.     InputA: in std_logic_vector(7 downto 0); --Preset states to nothing is "undefined"
    14.     InputB: in std_logic_vector(3 downto 0);
    15.     enable: in std_logic:='0';
    16.     Output: out std_logic_Vector(7 downto 0)
    17.     );
    18.  
    19. end muladder;
    20.  
    21. architecture multiplier of muladder is    
    22. Begin
    23.     process (klok, load, enable)
    24.         Variable Temp, Adda: std_logic_vector(7 downto 0); --Used variables to update them
    25.         Variable Active: std_logic:='0';                                                                    -- instantly, rather after the process
    26.         Variable InputBSub: std_logic_vector(3 downto 0);
    27.    
    28.  
    29. begin    
    30.  
    31. if (rising_edge(klok)) then
    32.     if load = '1' and enable = '0' and active = '0' then
    33.         Active := '1';
    34.         Temp:= InputA;     --loads Input A
    35.         Adda:=InputA;                                        --Loads input A to be added to itself
    36.         InputBSub:= InputB;                              --THe counter variable
    37.        
    38.         Output(3 downto 0)<= Temp(3 downto 0);
    39.         Output(7 downto 4)<= InputBSub;
    40.        
    41.     elsif (load = '0' and enable = '1' and active = '1') then
    42.         Output<="00000000";
    43.         Temp:= Temp+AddA;
    44.         InputBSub:= InputBSub - 1;
    45.         if(InputBSub = "0001") then
    46.                 Active  := '0';
    47.         end if;
    48.         OutPut<=Temp;
    49.     end if;
    50.    
    51.  
    52.        
    53. end if;
    54.  
    55.  
    56. end process;
    57.    
    58.  
    59. end multiplier;
    60.  
    61.  
     
  6. kubeek

    AAC Fanatic!

    Sep 20, 2005
    4,670
    804
    Do you use the xilinx ise? There should be an option to generate post-synthesis simulation and then view it. Also there is a post-place and route simulation
     
  7. WBahn

    Moderator

    Mar 31, 2012
    17,788
    4,808
    It has been well over a decade since I did anything with VHDL, so I am making some guesses at what I am seeing. But, assuming I am right, here is what I see.

    You have a number of signals that you define what they should be following each rising clock edge when {load,enable,active} = {100} or {011}. Well, what about the other six possible combinations? What should Output be then? What should Temp be then? What should Adda be then? For that matter, what should Adda be with you have {011}?

    The only thing the synthesizer can do is assume that they should remain unchanged. But that means that latches have to be inferred to make that happen. But is that what you want/need?

    The other thing is that you seem to be mixing blocking (using :=) and nonblocking (using <=) assignments. You need to be sure you understand these, at least in reasonable detail.

    Remember, you are not writing a program, you are describing physical hardware. The two are just enough alike to be dangerous.
     
  8. Zeppelin1007

    Thread Starter New Member

    Mar 6, 2012
    10
    0

    i do use ISE. I just used the "simulate behavioral model" for simulation.Ill look up how to do the post place/route simulation.
     
  9. WBahn

    Moderator

    Mar 31, 2012
    17,788
    4,808
    Avoid getting caught in the "design by simulation" trap. Many people make effectively random changes to their design until the simulation "looks good" and then call it a day. That is what one of my profs used to refer to as "a happening". Simulations are fine and wonderful, but don't them them do your thinking for you. Examine the sim output and determine what you believe to be the reason that your design results in that output. Then make a specific change that you believe will correct that output (or at least alter it is a way that you predict) and then see if that happens. If not, reanalyze the situation in light of the new knowledge.

    You final design should work because you designed it to work, not because you happen to come upon a set of statements that appear to simulate properly for the tiny set of possible input conditions the simulation covered.
     
  10. Zeppelin1007

    Thread Starter New Member

    Mar 6, 2012
    10
    0
    WBahn, funny you put it that way, our guy purposely lays "traps" for us..and this was one. Im sure he'll be laughing it up tuesday. But hey i learned alot from this prof. And from here.

    For starters, i found that making my inputA, 8 bits wide wasnt the thing to do. Make it 4. This caused the simulation to give me XXXXXXXX as an output. But i uploaded it to the board and guess what? Perfect!

    I spoke to the only other 2 guys that have it complete, and they replied the same thing. Simulation is questionable if it even worked, but it works on the board.
     
  11. WBahn

    Moderator

    Mar 31, 2012
    17,788
    4,808
    Your results in hardware should match your results in software. If they don't, then you really can't have confidence that your design is good (unless you can confidently explain WHY they differ and show that it is acceptable that they differ). Seeing a mix of blocking and non-blocking assignments is a huge red flag that should not be ignored. Seeing multiple assignments to the same signal as a result of a single clock event is a huge red flag that should not be ignored. Seeing lots of unexplained inferred latches is a huge red flag that should not be ignore.
     
Loading...