search

chipsalliance / Surelog

SystemVerilog 2017 Pre-processor, Parser, Elaborator, UHDM Compiler. Provides IEEE Design/TB C/C++ VPI and Python AST & UHDM APIs. Compiles on Linux gcc, Windows msys2-gcc & msvc, OsX
Apache License 2.0
368 stars 69 forks source link

Handling of "'1" expression #3375

Closed tcal-x closed 1 year ago

tcal-x commented 1 year ago

I have found a mismatch between Surelog and other tools in the handling of the RHS expression '1.

I haven't found a link to the SV spec, but I did find this relevant page: https://www.asic-world.com/systemverilog/literal_values1.html

The behavior of '1 described on that page, and that I've observed in other tools, is that it should expand to ALL 1 bits for the width of the usage. However, in Surelog, I see it being interpreted as just 1 (that's how I initially read it as well, as someone who knows Verilog but not SystemVerilog).

I have this as a testcase:

module top(output [3:0] x);
  assign x = '1;
endmodule

and this as a testbench:

module testbench;
wire [3:0] x;

    top U0(.x(x));
    initial begin
      #1 $display("%b", x);
    end     

endmodule

From iverilog direct simulation of the above files, using the -g2005-sv flag, I get this output, which I believe is correct:

1111

However, after feeding the testcase through Surelog, and converting back to Verilog using Yosys plugins, and simulating with the same testbench, I get this:

0001

The converted assignment that I believe is incorrect is this:

  assign x = 4'h1;

Using uhdm-dump, I see this for the RHS of the assign statement in the generated UHDM:

    |vpiRhs:
    \_constant: , line:3:12, endln:3:14
      |vpiParent:
      \_cont_assign: , line:3:8, endln:3:14
      |vpiDecompile:'1
      |vpiSize:-1
      |BIN:1
      |vpiConstType:3
tcal-x commented 1 year ago

(Do you know if this construct is covered in https://github.com/chipsalliance/sv-tests, @hzeller ?)

tcal-x commented 1 year ago

One more note: I originally hit this issue not in an assignment, but in a binary operator (==) using '1 as one of the operands:

assign p = product_exponent[7:0] == '1;
alaindargelas commented 1 year ago

I just fixed the original case. This is an entirely other ball game: assign p = product_exponent[7:0] == '1;

alaindargelas commented 1 year ago

BTW vpiSize:-1 means unsized. The plugin has some knowledge of what that means (Adapt to another expression size).

tcal-x commented 1 year ago

This is an entirely other ball game: assign p = product_exponent[7:0] == '1;

Yep. I wouldn't be surprised if it's a case where the other tools are going beyond what's in the spec.

alaindargelas commented 1 year ago

Fixed by

https://github.com/chipsalliance/Surelog/pull/3390

3390 by alaindargelas was merged 2 hours ago

https://github.com/chipsalliance/Surelog/pull/3389

3389 by alaindargelas was merged 5 hours ago

https://github.com/chipsalliance/Surelog/pull/3388

3388 by alaindargelas was merged yesterday

https://github.com/chipsalliance/Surelog/pull/3380

3380 by alaindargelas was merged 3 days ago

https://github.com/chipsalliance/UHDM/pull/841

841 by alaindargelas was merged 3 hours ago

https://github.com/chipsalliance/UHDM/pull/840

840 by alaindargelas was merged 5 hours ago

https://github.com/chipsalliance/UHDM/pull/838

838 by alaindargelas was merged 2 days ago

grotival commented 1 year ago

I bumped into a variation of this bug recently on Google's verilog:

typedef struct packed {logic [3:0] a;} my_struct_packed_t;

module unsized_single_bit_1 (
    output my_struct_packed_t out3
);

  // out3.a should be 4'b1111
  assign out3.a = '1;

endmodule : unsized_single_bit_1

out3.a becomes 4'b0001. Maybe the width of the struct packed is not properly read?

FYI: @tcal-x @hzeller @QuantamHD

grotival commented 1 year ago

To make sure everything is covered:

typedef struct packed {logic [3:0] a;} my_struct_packed_t;

module unsized_single_bit_1 (
    output wire [3:0] out1,
    output wire out2,
    output my_struct_packed_t out3,
    output wire out4
);

  // out1 should be 4'b1111
  assign out1   = '1;

  // out2 should be 1'b1
  assign out2   = (out1 == 4'b1111);

  // out3 should be 4'b1111
  assign out3.a = '1;

  // out4 should be 1'b1
  assign out4   = (out3 == '1);

endmodule : unsized_single_bit_1

Right now out4 is 1'b1. Which it should be if out3 had the right value. But per my previous comment it doesn't so there might be another bug piling right after the out3 one (or maybe that will be fixed at the same time).

alaindargelas commented 1 year ago

Surelog creates a correct UHDM model for all the assignments, it conservatively does not compute the '1 and leaves it at size -1 (unknown size). It creates correct expressions as you can see below. I'll see if Surelog can further help the plugin/Yosys to compute that value. It looks like a different pass is necessary. Ideally it could happen in the Yosys plugin.

====== UHDM =======
|uhdmtopModules:
\_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
  |vpiName:work@unsized_single_bit_1
  |vpiDefName:work@unsized_single_bit_1
  |vpiTop:1
  |vpiNet:
  \_logic_net: (work@unsized_single_bit_1.out1), id:43, line:4:23, endln:4:27
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiTypespec:
    \_logic_typespec: , id:39, line:4:12, endln:4:22
      |vpiRange:
      \_range: , id:36, line:4:17, endln:4:22
        |vpiLeftRange:
        \_constant: , id:37, line:4:18, endln:4:19
          |vpiParent:
          \_range: , id:36, line:4:17, endln:4:22
          |vpiDecompile:3
          |vpiSize:64
          |UINT:3
          |vpiConstType:9
        |vpiRightRange:
        \_constant: , id:38, line:4:20, endln:4:21
          |vpiParent:
          \_range: , id:36, line:4:17, endln:4:22
          |vpiDecompile:0
          |vpiSize:64
          |UINT:0
          |vpiConstType:9
    |vpiName:out1
    |vpiFullName:work@unsized_single_bit_1.out1
    |vpiNetType:1
  |vpiNet:
  \_logic_net: (work@unsized_single_bit_1.out2), id:45, line:5:17, endln:5:21
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiTypespec:
    \_logic_typespec: , id:44, line:5:12, endln:5:16
    |vpiName:out2
    |vpiFullName:work@unsized_single_bit_1.out2
    |vpiNetType:1
  |vpiNet:
  \_struct_net: (work@unsized_single_bit_1.out3), id:46, line:6:31, endln:6:35
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiTypespec:
    \_struct_typespec: (my_struct_packed_t), id:0, line:1:9, endln:1:15
    |vpiName:out3
    |vpiFullName:work@unsized_single_bit_1.out3
  |vpiNet:
  \_logic_net: (work@unsized_single_bit_1.out4), id:48, line:7:17, endln:7:21
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiTypespec:
    \_logic_typespec: , id:47, line:7:12, endln:7:16
    |vpiName:out4
    |vpiFullName:work@unsized_single_bit_1.out4
    |vpiNetType:1
  |vpiTopModule:1
  |vpiPort:
  \_port: (out1), id:94, line:4:23, endln:4:27
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiName:out1
    |vpiDirection:2
    |vpiLowConn:
    \_ref_obj: (work@unsized_single_bit_1.out1), id:95, line:4:23, endln:4:27
      |vpiParent:
      \_port: (out1), id:94, line:4:23, endln:4:27
      |vpiName:out1
      |vpiFullName:work@unsized_single_bit_1.out1
      |vpiActual:
      \_logic_net: (work@unsized_single_bit_1.out1), id:43, line:4:23, endln:4:27
    |vpiTypedef:
    \_logic_typespec: , id:30, line:4:12, endln:4:22
      |vpiRange:
      \_range: , id:26, line:4:17, endln:4:22
        |vpiParent:
        \_port: (out1), id:25, line:4:23, endln:4:27
        |vpiLeftRange:
        \_constant: , id:27, line:4:18, endln:4:19
          |vpiParent:
          \_range: , id:26, line:4:17, endln:4:22
          |vpiDecompile:3
          |vpiSize:64
          |UINT:3
          |vpiConstType:9
        |vpiRightRange:
        \_constant: , id:29, line:4:20, endln:4:21
          |vpiParent:
          \_range: , id:26, line:4:17, endln:4:22
          |vpiDecompile:0
          |vpiSize:64
          |UINT:0
          |vpiConstType:9
    |vpiInstance:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
  |vpiPort:
  \_port: (out2), id:96, line:5:17, endln:5:21
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiName:out2
    |vpiDirection:2
    |vpiLowConn:
    \_ref_obj: (work@unsized_single_bit_1.out2), id:97, line:5:17, endln:5:21
      |vpiParent:
      \_port: (out2), id:96, line:5:17, endln:5:21
      |vpiName:out2
      |vpiFullName:work@unsized_single_bit_1.out2
      |vpiActual:
      \_logic_net: (work@unsized_single_bit_1.out2), id:45, line:5:17, endln:5:21
    |vpiTypedef:
    \_logic_typespec: , id:32, line:5:12, endln:5:16
    |vpiInstance:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
  |vpiPort:
  \_port: (out3), id:98, line:6:31, endln:6:35
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiName:out3
    |vpiDirection:2
    |vpiLowConn:
    \_ref_obj: (work@unsized_single_bit_1.out3), id:99, line:6:31, endln:6:35
      |vpiParent:
      \_port: (out3), id:98, line:6:31, endln:6:35
      |vpiName:out3
      |vpiFullName:work@unsized_single_bit_1.out3
      |vpiActual:
      \_struct_net: (work@unsized_single_bit_1.out3), id:46, line:6:31, endln:6:35
    |vpiTypedef:
    \_struct_typespec: (my_struct_packed_t), id:0, line:1:9, endln:1:15
    |vpiInstance:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
  |vpiPort:
  \_port: (out4), id:100, line:7:17, endln:7:21
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiName:out4
    |vpiDirection:2
    |vpiLowConn:
    \_ref_obj: (work@unsized_single_bit_1.out4), id:101, line:7:17, endln:7:21
      |vpiParent:
      \_port: (out4), id:100, line:7:17, endln:7:21
      |vpiName:out4
      |vpiFullName:work@unsized_single_bit_1.out4
      |vpiActual:
      \_logic_net: (work@unsized_single_bit_1.out4), id:48, line:7:17, endln:7:21
    |vpiTypedef:
    \_logic_typespec: , id:35, line:7:12, endln:7:16
    |vpiInstance:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
  |vpiContAssign:
  \_cont_assign: , id:102, line:11:10, endln:11:21
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiRhs:
    \_constant: , id:78, line:11:19, endln:11:21
      |vpiParent:
      \_cont_assign: , id:75, line:11:10, endln:11:21
      |vpiDecompile:15
      |vpiSize:4
      |UINT:15
      |vpiConstType:9
    |vpiLhs:
    \_ref_obj: (work@unsized_single_bit_1.out1), id:103, line:11:10, endln:11:14
      |vpiParent:
      \_cont_assign: , id:102, line:11:10, endln:11:21
      |vpiName:out1
      |vpiFullName:work@unsized_single_bit_1.out1
      |vpiActual:
      \_logic_net: (work@unsized_single_bit_1.out1), id:43, line:4:23, endln:4:27
  |vpiContAssign:
  \_cont_assign: , id:104, line:14:10, endln:14:36
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiRhs:
    \_operation: , id:106, line:14:20, endln:14:35
      |vpiParent:
      \_cont_assign: , id:104, line:14:10, endln:14:36
      |vpiOpType:14
      |vpiOperand:
      \_ref_obj: (work@unsized_single_bit_1.out1), id:107, line:14:20, endln:14:24
        |vpiParent:
        \_operation: , id:106, line:14:20, endln:14:35
        |vpiName:out1
        |vpiFullName:work@unsized_single_bit_1.out1
        |vpiActual:
        \_logic_net: (work@unsized_single_bit_1.out1), id:43, line:4:23, endln:4:27
      |vpiOperand:
      \_constant: , id:83, line:14:28, endln:14:35
        |vpiParent:
        \_operation: , id:81, line:14:20, endln:14:35
        |vpiDecompile:4'b1111
        |vpiSize:4
        |BIN:1111
        |vpiConstType:3
    |vpiLhs:
    \_ref_obj: (work@unsized_single_bit_1.out2), id:105, line:14:10, endln:14:14
      |vpiParent:
      \_cont_assign: , id:104, line:14:10, endln:14:36
      |vpiName:out2
      |vpiFullName:work@unsized_single_bit_1.out2
      |vpiActual:
      \_logic_net: (work@unsized_single_bit_1.out2), id:45, line:5:17, endln:5:21
  |vpiContAssign:
  \_cont_assign: , id:108, line:17:10, endln:17:21
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiRhs:
    \_constant: , id:112, line:17:19, endln:17:21
      |vpiParent:
      \_cont_assign: , id:108, line:17:10, endln:17:21
      |vpiDecompile:'1
      |vpiSize:-1
      |BIN:1
      |vpiConstType:3
    |vpiLhs:
    \_hier_path: (out3.a), id:109, line:17:10, endln:17:16
      |vpiParent:
      \_cont_assign: , id:108, line:17:10, endln:17:21
      |vpiName:out3.a
      |vpiActual:
      \_ref_obj: (out3), id:110
        |vpiParent:
        \_hier_path: (out3.a), id:109, line:17:10, endln:17:16
        |vpiName:out3
        |vpiActual:
        \_struct_net: (work@unsized_single_bit_1.out3), id:46, line:6:31, endln:6:35
      |vpiActual:
      \_ref_obj: (a), id:111
        |vpiParent:
        \_hier_path: (out3.a), id:109, line:17:10, endln:17:16
        |vpiName:a
        |vpiActual:
        \_typespec_member: (a), id:6, line:1:36, endln:1:37
  |vpiContAssign:
  \_cont_assign: , id:113, line:20:10, endln:20:31
    |vpiParent:
    \_module_inst: work@unsized_single_bit_1 (work@unsized_single_bit_1), id:73, file:/home/alain/Surelog/tests/ScratchPad.sv, line:3:1, endln:22:33
    |vpiRhs:
    \_operation: , id:115, line:20:20, endln:20:30
      |vpiParent:
      \_cont_assign: , id:113, line:20:10, endln:20:31
      |vpiOpType:14
      |vpiOperand:
      \_ref_obj: (work@unsized_single_bit_1.out3), id:116, line:20:20, endln:20:24
        |vpiParent:
        \_operation: , id:115, line:20:20, endln:20:30
        |vpiName:out3
        |vpiFullName:work@unsized_single_bit_1.out3
        |vpiActual:
        \_struct_net: (work@unsized_single_bit_1.out3), id:46, line:6:31, endln:6:35
      |vpiOperand:
      \_constant: , id:118, line:20:28, endln:20:30
        |vpiDecompile:15
        |vpiSize:4
        |UINT:15
        |vpiConstType:9
    |vpiLhs:
    \_ref_obj: (work@unsized_single_bit_1.out4), id:114, line:20:10, endln:20:14
      |vpiParent:
      \_cont_assign: , id:113, line:20:10, endln:20:31
      |vpiName:out4
      |vpiFullName:work@unsized_single_bit_1.out4
      |vpiActual:
      \_logic_net: (work@unsized_single_bit_1.out4), id:48, line:7:17, endln:7:21
alaindargelas commented 1 year ago

3647 expands the value to 4'b1111

@tgorochowik, you'll need to bump Surelog in the plugin to benefit.

grotival commented 1 year ago

ah, I see! Thanks for taking a look so quickly! So I filed the bug at the wrong place, right? Should be on https://github.com/chipsalliance/yosys-f4pga-plugins?

alaindargelas commented 1 year ago

Correct, that will ensure proper follow-up from the plugin to Surelog (if the issue is there).