verilator icon indicating copy to clipboard operation
verilator copied to clipboard

Warn/Error code with X, Z values use

Open algrobman opened this issue 1 year ago • 26 comments

Does verilator warn/error RTL code with X or Z values usage? We use bunch of arithmetic modules and they have "if" statements with X values like

if(a==1) out = .. else if (a === 1'bx) out = 'x else out = ...

And because verilator is 2 state simulator it takes 'if X' branch for zero value of 'a' , producing wrong result at the end.

algrobman avatar May 11 '24 20:05 algrobman

right now we have: -Wno-WIDTH -Wno-MULTIDRIVEN -Wno-UNOPTFLAT -Wno-STMTDLY to disable some warnings ...

algrobman avatar May 11 '24 20:05 algrobman

Verilator shouldn't ever consider 0 === 1'bx to be true.

Note it does in a few limited cases know how to X propagate during elaboration, so if you e.g. have a parameter with a value of 1'bx, then PARAM === 1'bx will be true, even though Verilator doesn't do X in a general sense.

So to summarize, we need a small test case ;)

wsnyder avatar May 11 '24 21:05 wsnyder

Was VCD dumping changed since verilator 4..?

algrobman avatar May 12 '24 19:05 algrobman

I see some discrepancy with Xcelium. for following

logic[7:0] a=1, dec;
always @(posedge clk) begin
    a = a << 1;
end

image

why does 'a' start from 0?

(even originally I had a= 1; in initial statement, but it starts from 0, anyway?

algrobman avatar May 12 '24 19:05 algrobman

There were some dumping changes. Most likely there's a X=>0/1 transition at time zero causing the posedge to trigger. Thou shall use resets ;)

wsnyder avatar May 12 '24 20:05 wsnyder

Even if I define "a" as bit vector, nothing changes in waves and printing 'a' value at negedge clk . (xcellium starts 'a' as 1 , verilator as 0 , bug?

I expect that at first clk rising edge 'a' should become 2, not 1...

BTW, if 'a' would have initial value of 0, "a=a<<1;" statement would never change the 'a' .
(I do not have reset - I test combo logic and 'a' is input to it)

algrobman avatar May 12 '24 21:05 algrobman

I still suspect a time zero race between the initial and the assignment, with X's. Put a display in the always loop, and see how on Xcelium to get waves at each delta simulation point.

wsnyder avatar May 12 '24 21:05 wsnyder

BTW, to my original complain about no warnings for operations with X, Z. It's strange that the verilator 5.. started complain about # delays in the code, and does not complain about Xs ...

algrobman avatar May 12 '24 21:05 algrobman

I still suspect a time zero race between the initial and the assignment, with X's. Put a display in the always loop, and see how on Xcelium to get waves at each delta simulation point.

There should not be any race - the waves you see are done on code without initial statement. the Xcelliun waves look : image

algrobman avatar May 12 '24 21:05 algrobman

Because # delay behavior changed relative to older Verilators but X/Z are handled the same.

wsnyder avatar May 12 '24 21:05 wsnyder

Oh, you said "I see some discrepancy with Xcelium" I thought you meant the wrong values from Xcelium.

I'd still suggest put a display before and after the assign in the posedge.

wsnyder avatar May 12 '24 21:05 wsnyder

here $displays for Xcellium:

before a=01
after a=02
a=02, dec=02 enc=6 T=10
before a=02
after a=04
a=04, dec=04 enc=5 T=20
before a=04
..

Here for verilator:

VerilatorTB: Start of sim

a=01, dec=01 enc=7 T=10
before a=01
after a=02
a=02, dec=02 enc=6 T=20
before a=02
after a=04

its crazy - seems verilator misses 1st clk posedge (?)

(I generate clk by C++ code)

algrobman avatar May 12 '24 21:05 algrobman

statements with T= are $display at negadge clk ..

algrobman avatar May 12 '24 21:05 algrobman

As mentioned above, there's probably a X->0/1 clock edge in Xcelium, but Verilator won't get it. Avoid assuming edges on time 0 events, much better to rely on an explicit reset wire.

wsnyder avatar May 12 '24 22:05 wsnyder

looks like all this because of my C++ code:

int main(int argc, char** argv) {
  std::cout << "\nVerilatorTB: Start of sim\n" << std::endl;
        
  Verilated::commandArgs(argc, argv);

  Vtb_top* tb = new Vtb_top;

  // init trace dump
  VerilatedVcdC* tfp = NULL;
  
#if VM_TRACE
  Verilated::traceEverOn(true);
  tfp = new VerilatedVcdC;
  tb->trace (tfp, 24);
  tfp->open ("sim.vcd");
#endif
  // Simulate
  while(!Verilated::gotFinish()){
#if VM_TRACE
      tfp->dump (main_time);
#endif
      main_time += 5;
      tb->clk = !tb->clk;
      tb->eval();
  }

#if VM_TRACE
  tfp->close();
#endif

  std::cout << "\nVerilatorTB: End of sim" << std::endl;
  exit(EXIT_SUCCESS);

}

it does not evaluate design before the clock change at time 0 :(

algrobman avatar May 12 '24 22:05 algrobman

one more problem: Cadence/Synopsys tools could not read VCD produced by Verilator.

$version Generated by VerilatedVcd $end
$timescale 1ps $end
 $scope module  $end
  $scope module $unit $end
   $var wire 32 @G$ UVM_LOW [31:0] $end
  $upscope $end

I had to patch it as :

$version Generated by VerilatedVcd $end
$timescale 1ps $end
 $scope module **aaa**  $end
  $scope module **bbb** $end
   $var wire 32 @G$ UVM_LOW [31:0] $end
  $upscope $end

Cadence tool did not show waves, Synopsys DVE complained about line 4 illegal synthax:

Error: [VCD2VPD-PRINT-ERROR] VCD syntax error. A fatal error occurred at line 4: Syntax error - $end expected while reading $scope. The input VCD file contains unsupported syntax that cannot be converted to VPD. It could be that the input VCD was generated by a different simulator. Please check the VCD file and try again. Error: Conversion from VCD to VPD failed

I guess it was because no name in the $scope statements..

algrobman avatar May 13 '24 01:05 algrobman

Perhaps you're not creating the Verilated model with a name and that gets passed through? Anyhow, can you please submit a pull request to either name empty scopes, or (not sure this works) filter them out, or if you can't get to a pull, file a new issue with a self-contained test case showing the issue. Thanks.

wsnyder avatar May 13 '24 01:05 wsnyder

Perhaps you're not creating the Verilated model with a name and that gets passed through? Not sure I understand your question. My run script/TB/RTL content did not change. I did not see this problem with verilator 4.210, but with 5.024 ...

algrobman avatar May 13 '24 01:05 algrobman

There have been many tracing changes, if you get a bad file out of the master version, it's a bug.

wsnyder avatar May 13 '24 01:05 wsnyder

Wilson, thanks again. Now I got to our initial problem - xrun/verilator simulation difference with verilog checking for Xs:

here is the problematic module:

module lzd (a,enc,dec);

parameter a_width  = 8;

  parameter log_a_width = a_width > 536870912 ? 30 :
    (a_width > 268435456   ? 29 :
     (a_width > 134217728   ? 28 :
      (a_width > 67108864   ? 27 :
       (a_width > 33554432   ? 26 :
        (a_width > 16777216   ? 25 :
         (a_width > 8388608   ? 24 :
          (a_width > 4194304   ? 23 :
           (a_width > 2097152   ? 22 :
            (a_width > 1048576   ? 21 :
             (a_width > 524288    ? 20 :
              (a_width > 262144    ? 19 :
               (a_width > 131072    ? 18 :
                (a_width > 65536     ? 17 :
                 (a_width > 32768     ? 16 :
                  (a_width > 16384     ? 15 :
                   (a_width > 8192      ? 14 :
                    (a_width > 4096      ? 13 :
                     (a_width > 2048      ? 12 :
                      (a_width > 1024      ? 11 :
                       (a_width > 512       ? 10 :
                        (a_width > 256       ? 9 :
                         (a_width > 128       ? 8 :
                          (a_width > 64        ? 7 :
                           (a_width > 32        ? 6 :
                            (a_width > 16        ? 5 :
                             (a_width > 8         ? 4 :
                              (a_width > 4         ? 3 :
                               (a_width > 2 ? 2 : 1))))))))))))))))))))))))))));

input [a_width-1:0]  a;
output [log_a_width:0] enc;
output [a_width-1:0]  dec;
reg [log_a_width:0]  enc;
reg [a_width-1:0] dec;
integer i ;

always @ (a)
begin
    enc = {log_a_width+1{1'b1}};   // set enc default to all 1 assuming "A" is all zeros
    dec = {a_width{1'b0}};  // set dec default to all zeros
  begin : block
    for (i=0; (i < a_width); i=i+1) begin
      if (a[a_width-1-i] == 1'b1) begin
      $display("i=%0d", i);
        enc = i;
        dec[a_width-1-i] = 1'b1;
       disable block;  // found first "1", then stop looking
      end // if
      else if (a[a_width-1-i] == 1'bx) begin
      $display("X:i=%0d", i);
        enc = {log_a_width+1{1'bx}};
       disable block;  // when found first "x", then stop looking
      end // else if
    end // for
  end
end
endmodule

here is TB:

`ifdef VERILATOR
module tb_top ( input bit clk );
`else
module tb_top;
    bit                         clk;
`endif
parameter W = 8;
bit [W-1:0] a=1;
wire[W-1:0] dec;
logic [3:0] enc;


always @(negedge clk) begin
    $display("a=%h, dec=%h enc=%0d T=%0d", a, dec , enc, $time);
    if(a == 0) begin
        $finish;
    end
    // End Of test monitor
end

always @(posedge clk) begin
//    $display("before a=%h", a);
    a = a << 1;
//    $display("after a=%h", a);
end

// trace monitor

initial begin
//        a = $random;
`ifndef VERILATOR
        if($test$plusargs("dumpon")) $dumpvars;
        forever  clk = #5 ~clk;
`endif
end

lzd #W lzd(a,enc,dec);


endmodule

C++ wrapper was provided in previous comments.

xrun output :

i=7
i=6
a=02, dec=02 enc=6 T=10
i=5
a=04, dec=04 enc=5 T=20
i=4
a=08, dec=08 enc=4 T=30
i=3
a=10, dec=10 enc=3 T=40
i=2
a=20, dec=20 enc=2 T=50
i=1
a=40, dec=40 enc=1 T=60
i=0
a=80, dec=80 enc=0 T=70
a=00, dec=00 enc=15 T=80

verilator output:

VerilatorTB: Start of sim

X:i=0
X:i=0
a=02, dec=00 enc=0 T=10
X:i=0
a=04, dec=00 enc=0 T=20
X:i=0
a=08, dec=00 enc=0 T=30
X:i=0
a=10, dec=00 enc=0 T=40
X:i=0
a=20, dec=00 enc=0 T=50
X:i=0
a=40, dec=00 enc=0 T=60
i=0
a=80, dec=80 enc=0 T=70
X:i=0
a=00, dec=00 enc=0 T=80
- tb_top.sv:16: Verilog $finish

Based on that seems verilator evaluates if(a[i] == 1'bx) as TRUE and exits the for loop for a[i] ==0 .

Although xrun simulates this correctly.

That is why it's worth to warn compilation of RTL with Xs, at least

algrobman avatar May 13 '24 03:05 algrobman

There's a lot of code there, is what you intend to ask why:

module t;
   bit [7:0] a;
   initial begin
      a = 0;
      if (a[0] == 1'bx) $display("a=%x == 1'bx //BAD", a);
      if (a[0] != 1'bx) $display("a=%x != 1'bx", a);
      if (a[0] === 1'bx) $display("a=%x === 1'bx //BAD", a);
      if (a[0] !== 1'bx) $display("a=%x !== 1'bx", a);
      a = 1;
      if (a[0] == 1'bx) $display("a=%x == 1'bx //BAD", a);
      if (a[0] != 1'bx) $display("a=%x != 1'bx", a);
      if (a[0] === 1'bx) $display("a=%x === 1'bx //BAD", a);
      if (a[0] !== 1'bx) $display("a=%x !== 1'bx", a);
      $finish;
   end
endmodule

In Verilator should correctly handle or warn on:

a=00 == 1'bx //BAD
a=00 !== 1'bx
a=01 != 1'bx
a=01 !== 1'bx

I'd argue that any == 1'bx probably should most likely be === 1'bx, regardless of simulator, so that's a bit different than requesting a lint warning for Verilator only on every X/Z (which might also be a good idea).

wsnyder avatar May 13 '24 12:05 wsnyder

You are right about '===' , but we got this code from a third party and warn them about it. Regardless, X,Z usage warning could save a lot of debug time in future. I asked for global warning , thinking it easier to implement, than warn about specific cases. Another simulation discrepancy could be if X/Z are assigned. Besides, the warning can be always disabled by -Wno-*

algrobman avatar May 13 '24 14:05 algrobman

Are you going to fix all these issues?

algrobman avatar May 13 '24 20:05 algrobman

Are you going to fix all these issues?

By "all", I think we're down to general warning on = 'bx/z, and Verilator warning on cases where x/z may be dropped (which is not every assignment). The former is higher priority as is general and more likely problematic (and also what you hit).

By "you", I'm not personally going to get to either soon, pull requests welcome.

wsnyder avatar May 15 '24 14:05 wsnyder

I meant to add at least warnings for usage of X/Zs and fixing "empty/unnamed" $scope module statements in VCD any soon.

algrobman avatar May 15 '24 15:05 algrobman

As suggested above, file a separate ticket for $scope, including an example.

wsnyder avatar May 15 '24 17:05 wsnyder