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
366 stars 69 forks source link

Surelog doesn't set Ranges in the logic_net objects for wire commands #3562

Closed yurivict closed 1 year ago

yurivict commented 1 year ago

This Verilog code:

module top (y, wire1, wire2);
        input wire [7:0] wire1;
        input wire [7:0] wire2;
        output wire [15:0] y;

        wire signed [15:0] wire_merged;
        assign wire_merged = {wire1, wire2};

        assign y = wire_merged;
endmodule

has wire_merged defined with the range 15:0.

However, the surelog command surelog this.sv -parse -noelab -d uhdm prints the logic_net section for wire_merged that is missing the range specification:

  |vpiNet:
  \_logic_net: (work@top.wire_merged), line:6:21, endln:6:32
    |vpiParent:
    \_module_inst: work@top (work@top), file:/usr/home/yuri/nn-trainer/surelog-experiments/my-wire-before-use.sv, line:1:1, endln:10:10
    |vpiName:wire_merged
    |vpiFullName:work@top.wire_merged
    |vpiNetType:1

The generated UHDM file has the logic_net object for wire_merged, for which the Ranges() function returns null. This matches the fact that the above Surelog printout doesn't have the range.

The whole Surelog printout doesn't have the number 15 anywhere outside of the port specifications.

yurivict commented 1 year ago

Surelog loses ranges information for all wire [N1:N2] xx; statements.

Do you know what might be a fix or workaround for this?

This makes Surelog unusable as a convertor to the UHDM format.

Thomasb81 commented 1 year ago

The -d uhdm in combination with -noelab switch are a little bit miss leading.

-noelab ask for no elaboration : the uhdm db is not completed with additional information deduce from the source code : typically the uhdmtopmodules part is not produce.

On an other hand, It seems to me that the -d uhdm print only partial information of the uhdmallModules section.

It ends up you are not observing what you expect in the uhdm db. By removing the -noelab switch you should observe what you are looking for in the uhdmtopModules.

I would expect that if you perform your own walk of the allmodule uhdm. you would observe non null return value of vpiRange.

What I think is a bug is that wire signed [15:0] wire_merged; does not record the sign (vpiSign is absent in vpiNet)

surelog -nobuiltin -nocache -parse -d uhdm test_module.sv

module tb();

wire signed [15:0] test1;

endmodule
[...]
|uhdmtopModules:
\_module_inst: work@tb (work@tb), file:/home/tom/prog/git/Surelog/dbuild/test_module.sv, line:1:1, endln:5:10
  |vpiName:work@tb
  |vpiDefName:work@tb
  |vpiTop:1
  |vpiNet:
  \_logic_net: (work@tb.test1), line:3:20, endln:3:25
    |vpiParent:
    \_module_inst: work@tb (work@tb), file:/home/tom/prog/git/Surelog/dbuild/test_module.sv, line:1:1, endln:5:10
    |vpiTypespec:
    \_logic_typespec: , line:3:1, endln:3:19
      |vpiRange:
      \_range: , line:3:13, endln:3:19
        |vpiLeftRange:
        \_constant: , line:3:14, endln:3:16
          |vpiParent:
          \_range: , line:3:13, endln:3:19
          |vpiDecompile:15
          |vpiSize:64
          |UINT:15
          |vpiConstType:9
        |vpiRightRange:
        \_constant: , line:3:17, endln:3:18
          |vpiParent:
          \_range: , line:3:13, endln:3:19
          |vpiDecompile:0
          |vpiSize:64
          |UINT:0
          |vpiConstType:9
    |vpiName:test1
    |vpiFullName:work@tb.test1
    |vpiNetType:1
  |vpiTopModule:1
alaindargelas commented 1 year ago

@yurivict I'll work on this shortly. But as @Thomasb81 said, most of it is already correct, remove the -noelab, you need elab and look at the uhdmTopModules tree, not the uhdmAllModules, elaboration and sizing happens only in the elab tree.

yurivict commented 1 year ago

The -d uhdm in combination with -noelab switch are a little bit miss leading.

If -d uhdm can't produce a valid UHDM file with -noelab - it should just fail with a message that would lead the user in the right direction. For example:

error: can't write a valid UHDM file without Verilog elaboration, please consider not using the -noelab argument.

This would have resolved this confusion early on.

Thomasb81 commented 1 year ago

The uhdm db is just a container. To judge the content is no valid you should prove that data recorded inside does not respect systemVerilog LRM.

Not all tool need a complete uhdm db.

yurivict commented 1 year ago

@Thomasb81

By removing the -noelab switch you should observe what you are looking for in the uhdmtopModules.

On a side note, UHDM's VpiListener::listenDesigns() interface isn't flexible enough to allow to traverse only selected subtrees, like uhdmtopModules. It just always goes through all objects.

Thomasb81 commented 1 year ago

I understand that. I guess the default behavior of the VpiListener class is there to explore exhaustively the database for demonstration and test purpose. It would not be very efficient to visit details that does not interest you ...

What is preventing you to implement your own walk-through mechanism or derive VpiListener class and customize VpiListener::listenDesign to walk only through top module ?

You can look at listenDesign_ to only keep the part that interest you :

  if (vpiHandle itr = vpi_iterate(uhdmtopModules, handle)) {
    while (vpiHandle obj = vpi_scan(itr)) {
      listenAny(obj);
      vpi_free_object(obj);
    }
    vpi_free_object(itr);
  }

This way you skip all the top element of the uhdm db that do not interest you.

alaindargelas commented 1 year ago

@yurivict also, to add to @Thomasb81 perfectly valid approach, the class VpiListener has a member: isInUhdmAllIterator() that will let you know if you are in one of the "all" vs "top" objects.

Check the void VpiListener::listenDesign_(vpiHandle handle) method body in the generated code to pick and choose what you want to listen to.

alaindargelas commented 1 year ago

I'm working on fixing the "signed" property.

alaindargelas commented 1 year ago

Fixed by #3573