chipsalliance / synlig

SystemVerilog support for Yosys
Apache License 2.0
145 stars 19 forks source link

how to report issues #679

Open jeras opened 2 years ago

jeras commented 2 years ago

I would like to compile my SV code with yosys+UHDM, I followed the instructions from here: https://antmicro.com/blog/2022/02/simplifying-open-source-sv-synthesis-with-the-yosys-uhdm-plugin/ except I compiled the latest yosys master myself and installed it.

With the first source file I tried I got an error, the second file went well and the third failed again, the code was already tested in Quartus and Vivvado. Now I am not sure how to proceed.

  1. Is it ok if I used the script from the antmicro blog to install the plugin, or should I compile it from sources.
  2. Is is ok to use the yosys master, or should I use a branch/tag/hash?
  3. Does it make sense for me to report specific issues or is the project not there yet, and it would be better for me to wait.

I already created an issue here about 2 months ago: https://github.com/chipsalliance/UHDM-integration-tests/issues/670 Are this tests also running the yosys with the plugin? I can update this tests with my latest code. From the log it seems there were fewer issues then, than I have now. https://github.com/chipsalliance/Surelog/blob/master/third_party/tests/Rp32/rp32.log

I would try to get through more issues myself, but this projects is changing so fast I am not sure which git repo and instructions to look into. Also I lack experience with yosys and the opensource FPGA toolchain, so I will try to get through some examples before adding an extra layer of unknowns due to systemverilog.

Running script: https://github.com/jeras/rp32/blob/master/fpga/Xilinx/yosys/test.tcl failing on file/line: https://github.com/jeras/rp32/blob/master/hdl/rtl/riscv/riscv_isa_pkg.sv#L150

 Yosys 0.16+41 (git sha1 UNKNOWN, gcc 11.2.0-7ubuntu2 -fPIC -Os)

-- Executing script file `test.tcl' --

1. Executing Verilog with UHDM frontend.
[INF:CM0023] Creating log file ./slpp_all/surelog.log.

[WRN:PA0205] ../../../hdl/rtl/riscv/riscv_isa_pkg.sv:19: No timescale set for "riscv_isa_pkg".

[INF:CP0300] Compilation...

[INF:CP0301] ../../../hdl/rtl/riscv/riscv_isa_pkg.sv:19: Compile package "riscv_isa_pkg".

[INF:CP0302] Compile class "work@mailbox".

[INF:CP0302] Compile class "work@process".

[INF:CP0302] Compile class "work@semaphore".

[INF:EL0526] Design Elaboration...

[NTE:EL0508] Nb Top level modules: 0.

[NTE:EL0509] Max instance depth: 0.

[NTE:EL0510] Nb instances: 0.

[NTE:EL0511] Nb leaf instances: 0.

[INF:UH0706] Creating UHDM Model...

[  FATAL] : 0
[ SYNTAX] : 0
[  ERROR] : 0
[WARNING] : 1
[   NOTE] : 4
ERROR: ../../../hdl/rtl/riscv/riscv_isa_pkg.sv:150: Encountered unhandled operation type 69
tgorochowik commented 2 years ago

Hi

Thank you for your continued interest in the SystemVerilog plugin!

Is it ok if I used the script from the antmicro blog to install the plugin, or should I compile it from sources.

Both options should work the same way so you should be fine

Is is ok to use the yosys master, or should I use a branch/tag/hash?

We have yosys as a submodule in this repository and this is the version that is known to work properly (pass all the tests we have here), however as long as there are no breaking changes in mainline yosys using it should be fine (that is our goal anyway, to be able to use any yosys and just install our plugin).

Does it make sense for me to report specific issues or is the project not there yet, and it would be better for me to wait.

It certainly makes sense! Of course we cannot promise we will fix everything right away, but even if we don't, we would be very happy to know what our users are after! So all issues will be very appreciated

Looking at your logs, it looks as though you are doing everything correctly, the problem you encounter is that you use the wildcard operator ( ==?) in the line you link to: https://github.com/jeras/rp32/blob/master/hdl/rtl/riscv/riscv_isa_pkg.sv#L150

This is unfortunately not supported yet

jeras commented 2 years ago

I checked myself and figured the script from the blog is using the latest GitHub CI builds. So I will continue using the latest yosys and UHDM plugin builds.

I rewrote the if statements with the ==? operator into a priority casez statement and it seems to compile properly. I already had a workaround for a SV union, since they are also unsupported in Quartus.

Now I have issues with SystemVerolog packages. I checked the examples in the UHDM repo and noticed all the examples place the import between the module name and the port list. I was placing my imports outside the modules, which was not the best approach, but it worked in all the tools I used. Now I found the syntax from your examples in the standard, and I modified all my code.

I still get errors about packages not being found. The log below compiles only 2 files, the package and some RTL importing the package.

$ yosys -s test.tcl 

 /----------------------------------------------------------------------------\
 |                                                                            |
 |  yosys -- Yosys Open SYnthesis Suite                                       |
 |                                                                            |
 |  Copyright (C) 2012 - 2020  Claire Xenia Wolf <claire@yosyshq.com>         |
 |                                                                            |
 |  Permission to use, copy, modify, and/or distribute this software for any  |
 |  purpose with or without fee is hereby granted, provided that the above    |
 |  copyright notice and this permission notice appear in all copies.         |
 |                                                                            |
 |  THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES  |
 |  WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF          |
 |  MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR   |
 |  ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES    |
 |  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN     |
 |  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF   |
 |  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.            |
 |                                                                            |
 \----------------------------------------------------------------------------/

 Yosys 0.16+41 (git sha1 UNKNOWN, gcc 11.2.0-7ubuntu2 -fPIC -Os)

-- Executing script file `test.tcl' --

1. Executing Verilog with UHDM frontend.
[INF:CM0023] Creating log file ./slpp_all/surelog.log.

[WRN:PA0205] ../../../hdl/rtl/riscv/riscv_isa_pkg.sv:19: No timescale set for "riscv_isa_pkg".

[INF:CP0300] Compilation...

[INF:CP0301] ../../../hdl/rtl/riscv/riscv_isa_pkg.sv:19: Compile package "riscv_isa_pkg".

[INF:CP0302] Compile class "work@mailbox".

[INF:CP0302] Compile class "work@process".

[INF:CP0302] Compile class "work@semaphore".

[INF:EL0526] Design Elaboration...

[NTE:EL0508] Nb Top level modules: 0.

[NTE:EL0509] Max instance depth: 0.

[NTE:EL0510] Nb instances: 0.

[NTE:EL0511] Nb leaf instances: 0.

[INF:UH0706] Creating UHDM Model...

[  FATAL] : 0
[ SYNTAX] : 0
[  ERROR] : 0
[WARNING] : 1
[   NOTE] : 4

2. Executing Verilog with UHDM frontend.
[INF:CM0023] Creating log file ./slpp_all/surelog.log.

[WRN:PA0205] ../../../hdl/rtl/core/r5p_lsu.sv:19: No timescale set for "r5p_lsu".

[INF:CP0300] Compilation...

[INF:CP0303] ../../../hdl/rtl/core/r5p_lsu.sv:19: Compile module "work@r5p_lsu".

[INF:CP0302] Compile class "work@mailbox".

[INF:CP0302] Compile class "work@process".

[INF:CP0302] Compile class "work@semaphore".

[ERR:CP0316] ../../../hdl/rtl/core/r5p_lsu.sv:20: Undefined package "riscv_isa_pkg".

[INF:EL0526] Design Elaboration...

[NTE:EL0503] ../../../hdl/rtl/core/r5p_lsu.sv:19: Top level module "work@r5p_lsu".

[ERR:EL0528] ../../../hdl/rtl/core/r5p_lsu.sv:20: Undefined imported package: "riscv_isa_pkg".

[ERR:CP0317] ../../../hdl/rtl/core/r5p_lsu.sv:170: Undefined type "op32_l_func3_et".

[NTE:EL0508] Nb Top level modules: 1.

[NTE:EL0509] Max instance depth: 1.

[NTE:EL0510] Nb instances: 1.

[NTE:EL0511] Nb leaf instances: 0.

[INF:UH0706] Creating UHDM Model...

[  FATAL] : 0
[ SYNTAX] : 0
[  ERROR] : 3
[WARNING] : 1
[   NOTE] : 5
ERROR: Error when parsing design. Aborting!
rkapuscik commented 2 years ago

Thanks for reporting this!

We are using Surelog inside the plugin (the messages with square brackets in your output are actually from Surelog library). It performs the parsing and partial elaboration, then the plugin reads the (in-memory) UHDM output.

If you invoke read_systemverilog twice, you're actually running Surelog twice on the separate files. Since it performs elaboration, it means that all files that depend on each other need to be parsed together - this includes any imports. So the correct command in tcl would be

read_systemverilog ../../../hdl/rtl/riscv/riscv_isa_pkg.sv ../../../hdl/rtl/core/r5p_lsu.sv

We can also pass various arguments through this command to Surelog if needed (see its README). This could be useful for debugging, include files or some parameter overrides that need to be set.

Regarding the location of import statement: this should make no difference for the plugin, all of them should be supported. Feel free to open issues if that isn't the case.

jeras commented 2 years ago

After listing multiple source files in a single read_systemverilog call, I made a bit of progress with packages. But I still get generic errors like ERROR: Couldn't find ancestor for tagged pattern!.

I tried to compile only 2 source file, the first is a package, the second is a short RTL file. Here are the TCL script and the output log, I also attached the two source files.

plugin -i systemverilog
read_systemverilog \
  ../../../hdl/rtl/riscv/riscv_isa_pkg.sv \
  ../../../hdl/rtl/core/r5p_wbu.sv
$ yosys -s test.tcl 

 /----------------------------------------------------------------------------\
 |                                                                            |
 |  yosys -- Yosys Open SYnthesis Suite                                       |
 |                                                                            |
 |  Copyright (C) 2012 - 2020  Claire Xenia Wolf <claire@yosyshq.com>         |
 |                                                                            |
 |  Permission to use, copy, modify, and/or distribute this software for any  |
 |  purpose with or without fee is hereby granted, provided that the above    |
 |  copyright notice and this permission notice appear in all copies.         |
 |                                                                            |
 |  THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES  |
 |  WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF          |
 |  MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR   |
 |  ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES    |
 |  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN     |
 |  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF   |
 |  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.            |
 |                                                                            |
 \----------------------------------------------------------------------------/

 Yosys 0.16+41 (git sha1 UNKNOWN, gcc 11.2.0-7ubuntu2 -fPIC -Os)

-- Executing script file `test.tcl' --

1. Executing Verilog with UHDM frontend.
[INF:CM0023] Creating log file ./slpp_all/surelog.log.

[WRN:PA0205] ../../../hdl/rtl/riscv/riscv_isa_pkg.sv:19: No timescale set for "riscv_isa_pkg".

[WRN:PA0205] ../../../hdl/rtl/core/r5p_wbu.sv:19: No timescale set for "r5p_wbu".

[INF:CP0300] Compilation...

[INF:CP0301] ../../../hdl/rtl/riscv/riscv_isa_pkg.sv:19: Compile package "riscv_isa_pkg".

[INF:CP0303] ../../../hdl/rtl/core/r5p_wbu.sv:19: Compile module "work@r5p_wbu".

[INF:CP0302] Compile class "work@mailbox".

[INF:CP0302] Compile class "work@process".

[INF:CP0302] Compile class "work@semaphore".

[INF:EL0526] Design Elaboration...

[NTE:EL0503] ../../../hdl/rtl/core/r5p_wbu.sv:19: Top level module "work@r5p_wbu".

[NTE:EL0508] Nb Top level modules: 1.

[NTE:EL0509] Max instance depth: 1.

[NTE:EL0510] Nb instances: 1.

[NTE:EL0511] Nb leaf instances: 0.

[INF:UH0706] Creating UHDM Model...

[  FATAL] : 0
[ SYNTAX] : 0
[  ERROR] : 0
[WARNING] : 2
[   NOTE] : 5
ERROR: Couldn't find ancestor for tagged pattern!

I tried commenting out most of the RTL, to find out which code caused the error. If I commented out all ports/signals with a custom type fro the package, there was no error. I tried to explicitly write the source package for the type riscv_isa_pkg::ctl_t, this worked to some extent, but at least in few cases, it still caused an ERROR, if the package import was present. I did not try to modify all custom types and enumeration names.

A different behavior was where a custom type was used as a casting operator, in this case there was no warning or error, regardless whether the import was present or not. This could be a bug (missing check) or a check done at a later stage.

r5p.zip

jeras commented 2 years ago

Hi, I apologize for expecting too much help from you in the previous post. I made some progress, so I will try to list the issues I encountered.

Signal definition with initialization value

This is the probable cause of the issue from the previous report (actually similar const signals I removed from a SV package). The next code is a general purpose register file, with values initialized to zero (common approach for FPGA tools):

logic [XLEN-1:0] gpr [0:2**AW-1] = '{default: '0};

This created a yosys error (at least I think this are errors from yosys after syslog is done):

ERROR: Couldn't find ancestor for tagged pattern!

Parameter assignment containing expression

The code causing the error was at a parameter assignment for a module instance:

r5p_gpr #(
  .AW      (ISA.spec.base.E ? 4 : 5),
  ...
) gpr (
  ...
);

The reported error was:

../../../hdl/rtl/core/r5p_core.sv:296: ERROR: Identifier `\ISA' is implicitly declared and `default_nettype is set to none.

If I replaced the expression with a constant I got past this issue. The expression might not be the cause, it is possible the main issue was the complex hierarchical structure type of parameter ISA.

Slice of signal from SV interface

I wrote a partial address decoder, the system bus was a SV interface with modports:

    case (bus.adr[4-1:0])
      ...
    endcase

The error was:

../../../hdl/rtl/soc/r5p_soc_gpio.sv:115: ERROR: Failed to resolve identifier \bus.adr for width detection!

This error was not triggered, if I provided the top level module (-top r5p_core) while calling read_systemverilog (Surelog).

This is not really a bug, but the reason behind the error was not obvious to me initially. There should probably be some note in the documentation regarding this.

Interface array as module port

For a generic address decoder I used an array of SV interfaces with a modport as a module port.

  r5p_bus_if.man m[BN-1:0]   // system bus manager     ports (slave  devices connect here)

This is the error:

ERROR: :0: Encountered unhandled type in process_port: interface_array

I also used a SV interface array in a module, not as a port, and that instance did not trigger the same error, so it is possible only using it as a port is problematic.

Warnings regarding SV interfaces

This warnings imply something went wrong.

Warning: wire '\bus.rdt' is assigned in a block at ../../../hdl/rtl/soc/r5p_soc_mem.sv:57.0-57.0.
../../../hdl/rtl/soc/r5p_soc_mem.sv:65: Warning: Identifier `\bus.rdy' is implicitly declared.
../../../hdl/rtl/soc/r5p_soc_mem.sv:57: Warning: Identifier `\bus.rdt' is implicitly declared.
../../../hdl/rtl/soc/r5p_soc_mem.sv:47: Warning: Identifier `\bus.clk' is implicitly declared.
../../../hdl/rtl/soc/r5p_soc_mem.sv:48: Warning: Identifier `\bus.vld' is implicitly declared.
../../../hdl/rtl/soc/r5p_soc_mem.sv:49: Warning: Identifier `\bus.wen' is implicitly declared.
../../../hdl/rtl/soc/r5p_soc_mem.sv:51: Warning: Identifier `\bus.adr' is implicitly declared.
../../../hdl/rtl/soc/r5p_soc_mem.sv:51: Warning: Identifier `\bus.wdt' is implicitly declared.

Warning: Removing unused module

I am not sure why I get this warnings:

Warning: Removing unused module: \r5p_wbu from the design.
Warning: Removing unused module: \r5p_alu from the design.
Warning: Removing unused module: \r5p_bru from the design.
Warning: Removing unused module: \r5p_bus_arb from the design.
Warning: Removing unused module: \r5p_bus_dec from the design.
Warning: Removing unused module: \r5p_core from the design.
Warning: Removing unused module: \r5p_gpr from the design.
Warning: Removing unused module: \r5p_lsu from the design.
Warning: Removing unused module: \r5p_soc_gpio from the design.
Warning: Removing unused module: \r5p_soc_mem from the design.

They might disappear once I stop commenting out code that triggers errors (SV interface array).