# VHDL ALU, 8-bit register

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?

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;


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

• 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.) – TonyM Nov 1 '17 at 22:07
• What about that "synchronous reset"? Shouldn't your test for RES be inside the rising_edge test? – jonk Nov 1 '17 at 22:09
• I moved Q <= QA outside of the process but waveform seems incorrect still... – super95 Nov 1 '17 at 22:28
• 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. – MAB Nov 1 '17 at 22:29
• The second waveform diagram looks correct to me. Which part is wrong? – Entrepreneur Nov 1 '17 at 23:54

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.

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:

With this simulation:

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.

• How do Aa and Bb introduce DFFs? QA - yes. Aa and Bb are concurrent assignments. – TonyM Nov 2 '17 at 14:44
• @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. – Blair Fonville Nov 2 '17 at 14:52
• Dv answer because of the confusion in your wrong answer. Will remove dv once you've corrected the answer. – TonyM Nov 2 '17 at 16:11
• @TonyM Fair enough. Answer has been edited. Thanks for catching that. – Blair Fonville Nov 2 '17 at 19:21