Improve my “From NAND to Tetris” ALU in VHDL

Question

I'm following the course From NAND to Tetris, but instead of using the author's software, I'm trying to directly program a Spartan 6 FPGA. I'm now solving the ALU exercise, and ended up writing the following code (disclaimer: I started learning VHDL yesterday):

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;

entity alu is
    Port (x  : in  STD_LOGIC_VECTOR(15 downto 0);
          y  : in  STD_LOGIC_VECTOR(15 downto 0);

          zx : in  STD_LOGIC;
          nx : in  STD_LOGIC;
          zy : in  STD_LOGIC;
          ny : in  STD_LOGIC;
           f : in  STD_LOGIC;
          no : in  STD_LOGIC;

          output : out STD_LOGIC_VECTOR(15 downto 0);
          zr     : out STD_LOGIC;
          ng     : out STD_LOGIC);

             constant zeros: STD_LOGIC_VECTOR(15 downto 0) := "0000000000000000";
end alu;

architecture Behavioral of alu is
begin
    op: process(x, y, zx, nx, zy, ny, f, no)
      variable px: STD_LOGIC_VECTOR(15 downto 0);
      variable py: STD_LOGIC_VECTOR(15 downto 0);
      variable po: STD_LOGIC_VECTOR(15 downto 0);
    begin
      px := x;
      py := y;

      px := zeros   when (zx = '1');
      px := not(px) when (nx = '1');
      py := zeros   when (zy = '1');
      py := not(py) when (ny = '1');

      if (f = '1') then 
        po := (px + py);
      else
        po := (px AND py);
     end if;

      po := not(po) when (no = '1');
      output <= po;

     if (po = zeros) then
        zr <= '1';
      else
        zr <= '0';
      end if;

      ng <= po(0);
    end process;
end Behavioral;

I'm now wondering if there is a simpler way to achieve the same thing, considering:

1. (VHDL) Good practices;
2. (VHDL) Code size;
3. Using as little sequential logic as possible;
4. Synthesis size.

Answer 1

(VHDL) Good practices;

It's a lot easier meeting timing if you make each entity synchronous - give it a clock (and reset if necessary) and write a clocked process. You then also don't have to worry about keeping sensitivity lists up to date.

Note this:

use IEEE.STD_LOGIC_UNSIGNED.ALL;

Don't use that - although it's better behaved than std_logic_arith. Use ieee.numeric_std.all; instead and then use the appropriate types for your vectors.

Use of variables - that's a good sign in my book. Keeps local things very local.

You could size your variables automatically:

variable px : std_logic_vector(x'range);

and your constant (which you could move to the architecture, which would be more conventional):

constant zeros: STD_LOGIC_VECTOR(x'range) := (others => '0');

(VHDL) Code size

I'm not sure you could save that much code without losing readability

Using as little sequential logic as possible;

Why? Flipflops are almost free in FPGAs. I almost always run out of LUTs first.

Synthesis size

It's not often that the code has the biggest influence on synthesis size. It's higher level things like parallel (lots of logic) vs. sequential (a little logic in turn - but don't forget the state machine controlling it can be an overhead).