steveicarus / iverilog

Icarus Verilog
https://steveicarus.github.io/iverilog/
GNU General Public License v2.0
2.85k stars 530 forks source link

No error or warning reported when a vector lsb or msb value contains `x` or `z` bits #1140

Open Adivinedude opened 4 months ago

Adivinedude commented 4 months ago
module dummy_module_name #( parameter WIDTH = 4, parameter LATENCY = 0  )
    (
        input    wire    clk,
        output wire    out
    )
    localparam LP  = WIDTH / LATENCY * LATENCY == WIDTH ? WIDTH / LATENCY : WIDTH / LATENCY + 1;
    reg [7:0] register = 0;
    assign out = register == 0;
    always @(posedge clk) register <= register + 1'b1;
endmodule

When I run the above code and simulate with IVerlog, The only error message I get is

Starting Testbench with iVerilog
Finished Testbench
Task finished with errors exiting

There is no dump file or any output.

larsclausen commented 4 months ago

Division by 0 in Verilog results in a value of all 'x. Its not an error.

Adivinedude commented 4 months ago

for normal registers, I would agree. but this is being used to set a localparam, then to size a new vector. IVerilog gives no output, simply exits with the return value 1. No 'x' no nothing. the simulation fails to run.

martinwhitaker commented 4 months ago

The error messages you quote don't come from iverilog. Compiling your code with iverilog, I get

% iverilog test.v
test.v:6: syntax error
I give up.

That is because there is a missing semicolon after the module port list. Adding the semicolon allows the code to compile without error.

martinwhitaker commented 4 months ago

Closing as invalid. Feel free to reopen if you can provide a simple test case that demonstrates a problem when compiled with iverilog.

Adivinedude commented 4 months ago

This reproduces the issue. It just seams that something like this would produce a warning or error message. I guess its not a divide by zero issue, but an invalid msb issue

module top();
    localparam LP = 4'b000x;
    wire [LP:0] test_wires;
endmodule
caryr commented 4 months ago

If you can find anything in the standard that suggests something is being done incorrectly please let us know.

martinwhitaker commented 4 months ago

This reproduces the issue. It just seams that something like this would produce a warning or error message. I guess its not a divide by zero issue, but an invalid msb issue

OK, yes, this is a different issue to the one you originally reported. Your original example doesn't use LP, so there was no reason for the compiler to produce an error or a warning.

In traditional Verilog, the msb or lsb expressions are required to be integer expressions. The standard expression evaluation rules cause any x or z bits to be converted to 0 when converting a 4-state value to an integer, so strictly speaking this wouldn't be an error, although it would certainly warrant a warning. But SystemVerilog goes further and states that it is illegal for the msb or lsb expressions to contain any x or z bits, so yes, we should report an error when compiling SystemVerilog.

caryr commented 1 month ago

I have a fix for this specific issue, but there are still other places where an undefined/high-Z value in a constant is not being handled properly.

caryr commented 1 month ago

FYI I'm changing this to a bug since it is giving invalid/unexpected results without a warning.