8

I am trying to write a simple module to output a 14-bit number based on the value of four input signals. My attempt is shown below.

module select_size(
    input a,
    input b,
    input c,
    input d,
    output [13:0] size
);

    if (a) begin
        assign size = 14'h2222;
    end
    else begin
        if (b) begin
            assign size = 14'h1111;
        end
        else begin
            if (c) begin
                assign size = 14'h0777;
            end
            else begin
                assign size = 14'h0333;
            end
        end
    end

endmodule

Upon compilation, I receive the following error:

ERROR:HDLCompiler:44 - Line 67: c is not a constant

I don't understand why that particular if-statement isn't working if the other two preceding it are. I have tried changing the condition to

if (c == 1) begin

but to no avail.

Does anybody know how to solve this error? Thank you!

tomocafe
  • 1,425
  • 4
  • 20
  • 35
  • The compiler thinks those are conditional generate items, not conditional statements. –  Aug 07 '12 at 03:04

2 Answers2

14

Two problems:

1) You need to put if statements inside an always block.

If you use verilog-2001, you can use 

always @*
   if ....
   end
end

Otherwise specify all the inputs in the sensitivity list:

always @(a or b or c or d)
   if ....
   end
end

2) Constant assignments are not allowed inside if statements.

Remove the assign keyword from any statements inside the if block:

if (a) begin
    size = 14'h2222;
end

You will also have to declare size as a reg type.

However my preference would be to rewrite the entire module with conditional operator, I find it much preferrable to read. This following module achieves the same result:

module select_size(
    input a,
    input b,
    input c,
    input d,
    output [13:0] size
);

    assign size = a ? 14'h2222 :
                  b ? 14'h1111 :
                  c ? 14'h0777 : 
                      14'h0333 ;

endmodule
Tim
  • 35,413
  • 11
  • 95
  • 121
1

As @Tim has already answered, using reg types inside always blocks or wire with assign.

@Tim has also described the nested ternary assignments, while in the example are written very well, they are generally seen as bad practice. They imply a very long combinatorial path and can be hard to maintain. The combinatorial path may be optimised by synthesis which should imply a mux with optimised selection logic.

Easier to maintain code will have a lower cost of ownership, and as long as it does not lead to a larger synthesised design it is normally preferred.

My implementation would be to use a casez, (? are don't cares). I find the precedence of each value easier to see/debug.

module select_size(
  input a,
  input b,
  input c,
  input d,
  output logic [13:0] size //logic (SystemVerilog) or reg type
);

always @* begin
  casez ({a,b,c})
    3'b1?? : size = 14'h2222 ;
    3'b01? : size = 14'h1111 ;
    3'b001 : size = 14'h0777 ;
    3'b000 : size = 14'h0333 ;
    default: size = 'bx      ;
  endcase
end

endmodule
Morgan
  • 19,934
  • 8
  • 58
  • 84
  • "they are generally seen as bad practice". Do you have any source for this claim? I don't believe that it would necessarily generate any worse path than the case statement, given even trivial compiler optimization. – Tim Aug 08 '12 at 17:14
  • Also, while I know this is a trivial module, and somewhat of a design preference, in a more complex design I believe you should also add a default case which assigns the output to X. This is another reason why I prefer the ternary assignments, as if/else and case statement can have dangerous implications for X-Propagation in verification if you're not careful. If you simulate this and a/b/c/d are X, then the ternary assignment will propagate X to the output (ideal), the if/else will output 14'h0333, and the casez statement will latch the previous value since it has no default statement. – Tim Aug 09 '12 at 01:16
  • Generally bad practise, Every code review I have had/given. Company coding guide lines. My experience of maintenance on old code bases that use heavily nested ternary operators. Every one who mentors me has also made a point of avoiding them due to the headaches involved in debugging them when the do not work as expected. – Morgan Aug 09 '12 at 08:03
  • The Swallowing X's is a problem for debugging, as suggested by @Tim I have added a default statement. – Morgan Aug 09 '12 at 08:05
  • >"they are generally seen as bad practice". Do you have any source for this claim? Not sure my previous answer came across as intended. I have been influenced by the design preferences of those around me. In Ruby which also have the ternary operator I have seen in community code reviews that the use of nested operators obfuscated the code and lowered readability. For me it is a code readability issue. @Tim yours is the cleanest example of this I have seen, not every one is as careful with code formatting. – Morgan Aug 09 '12 at 08:28