8-Bit ALU in Verilog

I'm an EE student who's taken a a couple digital logic/design courses, but they were focused on schematic representation, so I'm teaching myself Verilog to implement what I've learned.

For a basic Hello World project, I've implemented a basic 8-bit ALU with addition/subtraction with carry/borrow, NOT, AND, OR, XOR, and carry/zero/sign flags.

I've implemented it on an FPGA and everything works, but I want to make sure I'm not falling for any beginner traps which can lead to dramatic/undesired results in the final implementation of the circuit.

module ALU(A, B, Cin, Op, Out, Cout, Zero, Sign);
    input[7:0] A, B;
    input Cin;
    //011-A+B, 111-A-B, 000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
    input[2:0] Op;
    output reg[7:0] Out;
    output Cout;
    output reg Zero;
    output reg Sign;

    wire FinalCin;          //Final value of Cin fed to ALU after XOR
    wire[7:0] FinalB;       //Final value of B fed to ALU after XOR
    wire[7:0] AddResult;
    wire[7:0] LogicResult;

    Adder DEV_ADD(A, FinalB, FinalCin, AddResult, Cout);
    LogicUnit DEV_LOG(A, B, Op, LogicResult);

    assign FinalCin = Cin ^ Op[2];  //Cin XOR Subtract Line
    assign FinalB = B ^ {8{Op[2]}}; //XOR B with Subtract Line

    always @ (Op or AddResult or LogicResult) begin
        if(Op == 3'b011 || Op == 3'b111)
            Out <= AddResult;
        else if(Op == 3'b000 || Op == 3'b001 || Op == 3'b010 || Op == 3'b100)
            Out <= LogicResult;
        else
            Out <= 8'b00000000;
    end

    always @ (Out) begin
        if(Out == 8'b00000000)
            Zero <= 1'b1;
        else
            Zero <= 1'b0;

        Sign <= Out[7];
    end
endmodule

Adder Module:

module Adder(A, B, Cin, S, Cout);
    input[7:0] A, B;
    input Cin;
    output wire[7:0] S;
    output wire Cout;

    assign {Cout, S} = A + B + Cin;

endmodule

Logic Unit:

```
module LogicUnit(A, B, Op, Out);
input[7:0] A, B;
input[2:0] Op; //000-NOT A, 001-A AND B, 01

Design Issue:

From a design perspective I see one issue in LogicUnit. Out is an inferred latch because it is not assigned in all conditions of Op. The risks associated with these latches and how to use them property (when needed) have already been discussed on other Stack Exchange sites; such as here on Electrical Engineering Stack Exchange and here on Stack Overflow. Synthesis considers all possibilities. Depending on your setup it may or may not recognize Op>4 as out of bounds. It is safer to give it an explicit assignment and allow the synthesizer to choose how to optimize.

In your situation a latch should not be inferred. The easiest way to resolve this is to ass a default statement:

always @* begin
  case(Op)
    3'b000: Out = ~A;
    3'b001: Out = A & B;
    3'b010: Out = A | B;
    3'b100: Out = A ^ B;
    default: Out = A ^ B; // <<-- or any expression to a known value
  endcase
end

You may have noticed I also changes the ` IEEE1800-2012(SystemVerilog)

module ALU( // PORT ORDER, DIRECTION, AND TYPE
  input [7:0] A, B,
  input Cin,
  //011-A+B, 111-A-B, 000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
  input [2:0] Op,
  output reg [7:0] Out,
  output Cout,
  output reg Zero,
  output reg Sign );

non-ANSI style (1995 style): IEEE1364-1995 ==> IEEE1800-2012(SystemVerilog)

module ALU(A, B, Cin, Op, Out, Cout, Zero, Sign); // PORT ORDER
  // PORT DIRECTION
  input [7:0] A, B;
  input Cin;
  //011-A+B, 111-A-B, 000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
  input [2:0] Op;
  output [7:0] Out;
  output Cout;
  output Zero;
  output Sign;
  // OUTPUT TYPE
  reg [7:0] Out;
  reg Zero;
  reg Sign;

non-ANSI style (2001 style): IEEE1364-2001 ==> IEEE1800-2012(SystemVerilog)

module ALU(A, B, Cin, Op, Out, Cout, Zero, Sign); // PORT ORDER
  // PORT DIRECTION AND TYPE
  input [7:0] A, B;
  input Cin;
  //011-A+B, 111-A-B, 000-NOT A, 001-A AND B, 010-A OR B, 100-A XOR B
  input [2:0] Op;
  output reg [7:0] Out;
  output Cout;
  output reg Zero;
  output reg Sign;

StackExchange Code Review Q#162147, answer score: 4