2
\$\begingroup\$

The following vhdl code is meant to do the operation as shown in the diagram.But the simulated waveform doesn't seem to be correct, I've spent hours but can't spot the mistake. Can someone help me please?

enter image description here

library ieee;
use ieee.std_logic_1164.all;


entity LU is
port(
CLK       :in  std_logic;                      -- System Clock
RES       :in  std_logic;                      -- Synchronous reset
OPCODE    :in  std_logic_vector(2 downto 0);   -- Logic Unit Opcode
A         :in  std_logic_vector(7 downto 0);   -- A input bus
B         :in  std_logic_vector(7 downto 0);   -- B input bus
Q         :out std_logic_vector(7 downto 0)    -- Q output from Logic Unit     
);

end LU;

architecture behavioral of LU is
signal QA,Aa,Bb : std_logic_vector(7 downto 0);

begin


Aa<=A;
Bb<=B;
process(clk)
begin

if RES = '1' then
  QA <= "00000000" ;
elsif rising_edge(clk) then
  case OPCODE is
        when "000" => 
            QA <= not(Aa);  
        when "001" => 
            QA <= not( Aa xor Bb);
        when "010" => 
            QA <= not(Aa or Bb); 
        when "011" => 
            QA <= not( Aa and Bb);
        when "100" => 
            QA <= Aa;              
        when "101" => 
            QA <= ( Aa xor Bb); 
        when "110" => 
            QA <= ( Aa or Bb);   
        when "111" => 
            QA <= ( Aa and Bb); 
        when others =>
            NULL;
    end case;
 Q<= QA ;
 end if ;
 end process ;
 end behavioral;

enter image description here

That's the wave after i tried moving Q <= QA assignment to outside of the process wave

\$\endgroup\$
  • \$\begingroup\$ Move your 'Q <= QA' assignment to outside of your process so its a concurrent statement. Where it is, it's implying a second set of DFFs that you don't want. (You don't actually need QA, you can more tidily assign directly to Q in your process. Similar thing goes for Aa and Bb.) \$\endgroup\$ – TonyM Nov 1 '17 at 22:07
  • \$\begingroup\$ What about that "synchronous reset"? Shouldn't your test for RES be inside the rising_edge test? \$\endgroup\$ – jonk Nov 1 '17 at 22:09
  • \$\begingroup\$ I moved Q <= QA outside of the process but waveform seems incorrect still... \$\endgroup\$ – super95 Nov 1 '17 at 22:28
  • \$\begingroup\$ This seems eerily familiar. Do you happen to study at the EPFL? I would suggest separating combinatorial from sequential logic in 2 different process. It usually clarifies things. \$\endgroup\$ – MAB Nov 1 '17 at 22:29
  • \$\begingroup\$ The second waveform diagram looks correct to me. Which part is wrong? \$\endgroup\$ – Entrepreneur Nov 1 '17 at 23:54
7
\$\begingroup\$

The problem that you're seeing is due to the unnecessary stage that your signal QA generates - declared here:

signal QA,Aa,Bb : std_logic_vector(7 downto 0);

Your OPCODE selects an assignment for QA on one clock cycle which, in turn, assigns the final output Q on the following clock cycle.

A cleaner approach would be to eliminate all of the internal signals, which are altogether unnecessary for your LU entity.

Try this instead:

library ieee;
use ieee.std_logic_1164.all;

entity LU is
port(
    CLK       :in  std_logic;                      -- System Clock
    RES       :in  std_logic;                      -- Synchronous reset
    OPCODE    :in  std_logic_vector(2 downto 0);   -- Logic Unit Opcode
    A         :in  std_logic_vector(7 downto 0);   -- A input bus
    B         :in  std_logic_vector(7 downto 0);   -- B input bus
    Q         :out std_logic_vector(7 downto 0));  -- Q output from Logic Unit     
end LU;

architecture behavioral of LU is
begin
    process(clk)
    begin
        if rising_edge(clk) then
            if RES = '1' then
                Q <= (others => '0');
            else
              case OPCODE is
                    when "000" => 
                        Q <= not(A);  
                    when "001" => 
                        Q <= not(A xor B);
                    when "010" => 
                        Q <= not(A or B); 
                    when "011" => 
                        Q <= not(A and B);
                    when "100" => 
                        Q <= A;              
                    when "101" => 
                        Q <= (A xor B); 
                    when "110" => 
                        Q <= (A or B);   
                    when "111" => 
                        Q <= (A and B); 
                    when others =>
                        Q <= (others => '0');
                end case;
            end if;
         end if;
     end process;
 end behavioral;

Which produces the correct RTL: Correct RTL

With this simulation: Correct simulation


Your VHDL yields this RTL: Incorrect RTL

There are three things to notice here:

  1. Your extra QA signal ends up generating an undesired 2nd stage of gates
  2. The 2nd stage of gates is improperly enabled. Notice that your RES signal ends up acting as an inverted clock enable on the final gates? This is due to the fact that your final output Q is not assigned a value during your RES = 1 condition. Your code only actually assigns Q within the elsif rising_edge(clk) then clause. This leads to a poor design. If a signal is assigned a value in one case (or if-then level), it should be assigned a value in all of them. Otherwise you will end up with latches or inappropriate CE signals.
  3. You should use synchronous resets (as I did in the above code). Short of that, a good convention to use is
    • Name the signal CLR for asynchronous operations
    • Name the signal RST for synchronous operations

Notice, that this is the convention employed by Xilinx, as you can confirm by comparing the two RTL diagrams.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ How do Aa and Bb introduce DFFs? QA - yes. Aa and Bb are concurrent assignments. \$\endgroup\$ – TonyM Nov 2 '17 at 14:44
  • \$\begingroup\$ @TonyM They don’t. I avoided specifically stating which signal created the DFFs in attempt to encourage the OP to simplify. But you’re right; I should probably clean up the wording. \$\endgroup\$ – Blair Fonville Nov 2 '17 at 14:52
  • \$\begingroup\$ Dv answer because of the confusion in your wrong answer. Will remove dv once you've corrected the answer. \$\endgroup\$ – TonyM Nov 2 '17 at 16:11
  • 1
    \$\begingroup\$ @TonyM Fair enough. Answer has been edited. Thanks for catching that. \$\endgroup\$ – Blair Fonville Nov 2 '17 at 19:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.