Does this Verilog HDL code of a decoder look strange in any way?

Question

I am wondering if it is ok to use output instead of wire for another output in Verilog coding (using Quartus for this). Just like in this example code where the output F1 is a result of a 4-input NAND gate, and the inputs for this NAND gate are 4 other outputs instead of wires.

The code compile without problem, but I am not sure if this is fine to code this way compared to using wires as the inputs for the NAND gate. Any reply will be appreciated.

module Decoder (x, y, z, D0, D1, D2, D3, D4, D5, D6, D7, F1, F2);
 input x, y, z;
 output D0, D1, D2, D3, D4, D5, D6, D7, F1, F2;
 
 assign D0 = ~x&~y&~z;
 assign D1 = ~x&~y&z;
 assign D2 = ~x&y&~z;
 assign D3 = ~x&y&z;
 assign D4 = x&~y&~z;
 assign D5 = x&~y&z;
 assign D6 = x&y&~z;
 assign D7 = x&y&z;
 
 assign F1 = ~(~D3&~D5&~D6&~D7);
 assign F2 = ~(~D1&~D2&~D4&~D7);
 
endmodule

Answer 1

Unless declared otherwise, an output is defined as a wire (*). So you can use it exactly like any other wire.

As to whether this is "strange", that depends entirely on what you consider strange. It is perfectly normal for an output wire to be used internally as well, though some people prefer to have an internal wire used internally, and the output assigned to that where needed, such as:

output myOut;
wire intWire;
assign myOut = intWire;

In some scenarios this can make things clearer to follow. It doesn't increase resource counts, wires are just net connections after all, so there is no downside to doing this.

There are of course cases where you should use an internal wire and not an output - the when is simple. Just ask yourself the question: do you need the signal (e.g D0, D1, etc.) outside of this module? If you need them, output is fine. If you don't need them, then they should not be declared as outputs.

---

(*) You will sometimes see people write this explicitly as output wire, though this is implied. You might also see output reg if a reg type was desired instead (same goes for other types).

---

As a side note, your input/output list would be much neater and easier to use if you swapped to using signal vectors rather than individual bits. Consider instead:

module Decoder (x,y,z,D,F);
input x,y,z;
output [7:0] D;
output [1:0] F;
assign D[0] = ...
assign D[1] = ...

This looks much cleaner and is easier to connect up.

I would also advise using the more recent (Verilog 2001 style) connection lists if you can rather than the original 95 versions as it avoids unnecessary duplication:

module Decoder (
 input x, y, z,
 output [7:0] D,
 output [1:0] F
);
assign D[0] = ...
assign D[1] = ...

---

As another aside, albeit a subjective one, you can use concatenation and reduction operators to neaten your final lines (or at least I find it more readable):

assign F1 = ~&( ~{D3,D5,D6,D7} );

This concatenates the four inputs, takes the bitwise not, and then performs the NAND operation across all bits.

---

It also seems your logic for D0-D7 is just an simple decoder of {x,y,z} == index, which can be written with much clarity as:

wire [2:0] xyz;
assign xyz = {x,y,z};
assign D0 = (xyz == 3'd0);
assign D1 = (xyz == 3'd1);
...
assign D7 = (xyz == 3'd7);

The benefit of using vectors for Dx then becomes apparent, as you can then start using loops to avoid having to write this list out in full. Either using a procedural block (still combinational logic):

output reg [7:0] D,
...
integer i;
assign xyz = {x,y,z};
always @ * begin
 for (i = 0; i < 8; i = i + 1) begin
 D[i] = (xyz == i[2:0]);
 end
end

Or even without the loop:

output reg [7:0] D,
...
assign xyz = {x,y,z};
always @ * begin
 D = 8'b0;
 D[xyz] = 1'b1;
end

Or using a generate block:

output [7:0] D,
...
genvar i;
assign xyz = {x,y,z};
generate for (i = 0; i < 8; i = i + 1) begin : xyz_decode
 assign D[i] = (xyz == i);
end endgenerate

The generate technique becomes very powerful once you get to know it.

Answer 2

The code was totally compilable without problem

You have taken the first step towards answering your question as to whether the Verilog code is fine. You used the Quartus Verilog compiler tool to check if there are any syntax errors, and you discovered that the syntax is fine.

The next step with Verilog is to run a simulation to see if it behaves as desired. To do so, you can create testbench code to run through all 8 values of the input, dump a waveform database then check the output waveforms.

module tb;
 reg x, y, z;
 wire D0, D1, D2, D3, D4, D5, D6, D7, F1, F2;
Decoder dut (
 // Inputs:
 .x (x),
 .y (y),
 .z (z),
 // Outputs:
 .D0 (D0),
 .D1 (D1),
 .D2 (D2),
 .D3 (D3),
 .D4 (D4),
 .D5 (D5),
 .D6 (D6),
 .D7 (D7),
 .F1 (F1),
 .F2 (F2)
);
initial begin
 $dumpvars;
 for (int i=0; i<8; i++) begin
 #5 {x,y,z} = i;
 end
 #5 $finish;
end
endmodule

---

waves

The simulation runs without errors or warnings, and the decoder outputs (D) look good to me. Tools help us to determine whether the code is fine.

There are many different ways to code similar logic in Verilog. As this shows, it is not necessary to declare wires in your decoder module. It is sometimes a matter of preference. If you are looking for a review of your working code, the Stack Exchange network has a site dedicated to that: CodeReview