status of support for SV unions

[jeras]

I would like to ask what is the state of support for SV union with the git versions of the plugin and yosys. For now I can see there is no outright error stating unions are not supported as was there when I last tried a couple months ago. I would like to know whether I should report SV union related bugs now, or should I wait a couple of months for it to get into a better shape. I created an example test here: https://github.com/jeras/UHDM-tests

While running the test I could not observe the same errors as in the original larger code, so I suspect there are multiple issues. For this reason I will not spend time yet for a report involving the full code, unless I am told union support is supposed to be in a working shape.

In previous runs I got the next errors:

The code line is not the same as in the example test, but it points to the u element of the union. All elements of the union are supposed to have 32-bits (RISC-V base I instructions).

../../../hdl/rtl/riscv/riscv_isa_pkg.sv:235: ERROR: member u of a packed union has 8 bits, expecting 11

After commenting out all but element i in the union I got a more verbose output. Note here many ranges do not make sense, are too short str='imm_11_0' logic basic_prep range=[9:9].

struct >AST_UNION <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:240.0-240.0> [0x56416ed2ead0] basic_prep
struct >  AST_STRUCT <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:232.0-232.0> [0x56416ed2dab0] str='i' basic_prep range=[9:0]
struct >    ATTR \packed_ranges:
struct >      AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed1ed60] bits='1'(1) basic_prep range=[0:0] int=1
struct >    ATTR \unpacked_ranges:
struct >      AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed2a340] bits='1'(1) basic_prep range=[0:0] int=1
struct >    AST_STRUCT_ITEM <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:232.0-232.0> [0x56416ed252f0] str='imm_11_0' logic basic_prep range=[9:9]
struct >      ATTR \packed_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed23910] bits='1'(1) basic_prep range=[0:0] int=1
struct >          AST_RANGE <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:232.0-232.0> [0x56416ed34120] basic_prep range=[11:0]
struct >            AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed22b10] bits='00000000000000000000000000001011'(32) signed basic_prep range=[31:0] int=11
struct >            AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed26fd0] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0]
struct >      ATTR \unpacked_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed30280] bits='1'(1) basic_prep range=[0:0] int=1
struct >    AST_STRUCT_ITEM <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:232.0-232.0> [0x56416ed30800] str='rs1' logic basic_prep range=[8:8]
struct >      ATTR \packed_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed2fba0] bits='1'(1) basic_prep range=[0:0] int=1
struct >          AST_RANGE <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:232.0-232.0> [0x56416ed30120] basic_prep range=[4:0]
struct >            AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed2f4c0] bits='00000000000000000000000000000100'(32) signed basic_prep range=[31:0] int=4
struct >            AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed2fa40] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0]
struct >      ATTR \unpacked_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed2f360] bits='1'(1) basic_prep range=[0:0] int=1
struct >    AST_STRUCT_ITEM <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:232.0-232.0> [0x56416ed30960] str='func3' logic basic_prep range=[7:7]
struct >      ATTR \packed_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed38730] bits='1'(1) basic_prep range=[0:0] int=1
struct >          AST_RANGE <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:172.0-172.0> [0x56416ed382d0] basic_prep range=[2:0]
struct >            AST_CONSTANT <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:172.0-172.0> [0x56416ed37ec0] bits='00000000000000000000000000000010'(32) signed basic_prep range=[31:0] int=2
struct >            AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed31040] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0]
struct >      ATTR \unpacked_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed315c0] bits='1'(1) basic_prep range=[0:0] int=1
struct >    AST_STRUCT_ITEM <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:232.0-232.0> [0x56416ed380e0] str='rd' logic basic_prep range=[6:6]
struct >      ATTR \packed_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed39c20] bits='1'(1) basic_prep range=[0:0] int=1
struct >          AST_RANGE <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:232.0-232.0> [0x56416ed391c0] basic_prep range=[4:0]
struct >            AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed39810] bits='00000000000000000000000000000100'(32) signed basic_prep range=[31:0] int=4
struct >            AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed393b0] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0]
struct >      ATTR \unpacked_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed38950] bits='1'(1) basic_prep range=[0:0] int=1
struct >    AST_STRUCT <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:175.0-175.0> [0x56416ed38fa0] str='opcode' basic_prep range=[5:0]
struct >      ATTR \packed_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed19a40] bits='1'(1) basic_prep range=[0:0] int=1
struct >      ATTR \unpacked_ranges:
struct >        AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed15470] bits='1'(1) basic_prep range=[0:0] int=1
struct >      AST_STRUCT_ITEM <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:176.0-176.0> [0x56416ed38b40] str='opc' basic_prep range=[5:1]
struct >        ATTR \packed_ranges:
struct >          AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed32eb0] bits='1'(1) basic_prep range=[0:0] int=1
struct >        ATTR \unpacked_ranges:
struct >          AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed32cc0] bits='1'(1) basic_prep range=[0:0] int=1
struct >      AST_STRUCT_ITEM <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:177.0-177.0> [0x56416ed33b80] str='c11' logic basic_prep range=[0:0]
struct >        ATTR \packed_ranges:
struct >          AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed3aa50] bits='1'(1) basic_prep range=[0:0] int=1
struct >            AST_RANGE <../../../hdl/rtl/riscv/riscv_isa_pkg.sv:177.0-177.0> [0x56416ed33720] basic_prep range=[1:0]
struct >              AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed2a190] bits='00000000000000000000000000000001'(32) signed basic_prep range=[31:0] int=1
struct >              AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed33530] bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0]
struct >        ATTR \unpacked_ranges:
struct >          AST_CONSTANT <../../../hdl/rtl/core/r5p_core.sv:0.0-0.0> [0x56416ed19900] bits='1'(1) basic_prep range=[0:0] int=1
ERROR: Couldn't find search elem: r in struct

[alaindargelas]

I just ran this example through the new equivalence checker that checks SV2V+Yosys parser against Surelog/UHDM front-end and got a PASS. Meaning the gate-level netlists post Yosys Synthesis are equivalent.

Save your test in a single file (put both package and module in same file for formal verification test harness reasons) under: UHDM-integration-tests/tests/UnionCast/dut.sv

Then run:

./run_formal_verif.sh test=UnionCast_dut.sv

| UnionCast_dut.sv | PASS | 0 | 0 | 0 | 1 | 0 | 1s | 223 | +------------------+----------+--------+----------+----------+----------+-----------+----------+----------+

You can inspect the formal run and the generated artifacts under: yosys-uhdm-plugin-integration$ ls ./build/tests/UnionCast_dut.sv/ equiv.out equiv.ys surelog_gate.v surelog.out surelog.ys sv2v.v UnionCast_dut.sv.log UnionCast_dut.sv.time yosys_gate.v yosys.out yosys.ys

"surelog_gate.v" is the gate level generated with the read_systemverilog front-end.

You might have to

• update the whole repo,
• build all the executables/libraries,
• then checkout the pending PR: https://github.com/antmicro/yosys-uhdm-plugin-integration/pull/935

[alaindargelas]

I inspected visually the UHDM dump and it looks correct too. I'll let @koluckirafal @kbieganski comment on the Yosys plugin layer.

[jeras]

Hi, this will take me some time and I am not sure I will not give up.

In the meantime I updated to the latest released version of the plugin and I got this: https://github.com/antmicro/yosys-uhdm-plugin-integration/issues/949

[jeras]

Hi, I am now using yosys and the plugins compiled as part of a yosys-uhdm-plugin-integration build, without installing it. I removed the plugins installed on the system just in case. I do not need the LD_LIBRARY_PATH variable apparently. So I should be running the same code as you.

I first got the code in the test to compile successfully as it did for you, the issue was, I forgot to rename the module I used as top.

In the next step I added more code from my original project and I was able to reproduce the issue.

$ $HOME/VLSI/yosys-uhdm-plugin-integration/image/bin/yosys -s build.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+65 (git sha1 52d8ddee0, gcc 9.4.0-1ubuntu1~20.04.1 -fPIC -Os)

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

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

[WRN:PA0205] riscv_isa_pkg.sv:19:1: No timescale set for "riscv_isa_pkg".

[WRN:PA0205] riscv_isa_c_pkg.sv:19:1: No timescale set for "riscv_isa_c_pkg".

[WRN:PA0205] test_union.sv:19:1: No timescale set for "test_union".

[INF:CP0300] Compilation...

[INF:CP0301] riscv_isa_pkg.sv:19:1: Compile package "riscv_isa_pkg".

[INF:CP0301] riscv_isa_c_pkg.sv:19:1: Compile package "riscv_isa_c_pkg".

[INF:CP0303] test_union.sv:19:1: Compile module "work@test_union".

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

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

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

[INF:EL0526] Design Elaboration...

[NTE:EL0503] test_union.sv:19:1: Top level module "work@test_union".

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

[NTE:EL0509] Max instance depth: 1.

[NTE:EL0510] Nb instances: 1.

[NTE:EL0511] Nb leaf instances: 1.

[INF:UH0706] Creating UHDM Model...

[  FATAL] : 0
[ SYNTAX] : 0
[  ERROR] : 0
[WARNING] : 3
[   NOTE] : 5
riscv_isa_pkg.sv:111: ERROR: member u of a packed union has 8 bits, expecting 11

I noticed two conditions are needed for this issue to be triggered:

1. riscv_isa_pkg contains a function using the unions defined before,
2. riscv_isa_c_pkg imports riscv_isa_pkg but can otherwise be empty.

Steps to reproduce the issue:

git clone https://github.com/jeras/UHDM-tests
cd UHDM-tests
../yosys-uhdm-plugin-integration/image/bin/yosys -s build.tcl

[alaindargelas]

I can reproduce the issue, there seems to be a strict condition in the UHDM->Yosys plugin that triggers that error. We will be looking at this shortly.

[alaindargelas]

@tgorochowik , please check in the plugin code why there is this error condition. I'll check the UHDM tree.

[pgielda]

I am guessing you wanted to mention someone else @alaindargelas? @tomaszgielda is not involved in this effort. FYI @tgorochowik @kgugala

[tgorochowik]

@jeras The error you're getting is pretty weird, but unions in general are supported.

There was one issue where we skipped a simplify pass for packed unions (fixed in https://github.com/chipsalliance/yosys-f4pga-plugins/pull/375) - but it shouldn't affect your code.

The issue you're getting seems to be caused by the empty package you added to your design.

With the following patch I am able to synthesize your design correctly.

diff --git a/riscv_isa_c_pkg.sv b/riscv_isa_c_pkg.sv
index ef9889d..2636bf9 100644
--- a/riscv_isa_c_pkg.sv
+++ b/riscv_isa_c_pkg.sv
@@ -18,6 +18,5 @@

 package riscv_isa_c_pkg;

-import riscv_isa_pkg::*;

 endpackage: riscv_isa_c_pkg

(or just remove riscv_isa_c_pkg from build.tcl as it's not used anyway).

This of course needs to be investigated and fixed, but to answer your question: unions are supported.

[jeras]

Thanks for looking into it. I am finally making some progress compiling my SystemVerilog RTL with yosys+UHDM. Before I went through 3 full cycles of 1 week of struggling and 2 months of giving up. This is the first UHDM issue, where the error was not on my side, otherwise I was just begging for help.

I could have recoded the RTL to avoid language support issues, but I have spent a couple of years reporting SystemVerilog related issues for Verilator, and now I feel compelled to do the same for yosys+UHDM. The end result being great open source SystemVerilog RTL support. I am also looking forward to Verilator 5.0 with support for # delays and future UVM support.

The package is empty only for the purpose of this bug report, I removed a lot of code not essential for reproducing the issue. The original would not compile without the import, since it contains code depending on it. Since this package contains optional RISC-V compressed extension code, I might try to get further with synthesis without it. But I made it a point to use many SV RTL features, so I expect to encounter other issues. Especially with my definition of the CSR address space, which is a large sparsely populated array, and might cause scalability issues in yosys. In that case I am not sure the sparse array coding style would be worth the trouble, if not for anything else, due to all the warnings about unused array elements being optimize out by synthesis.

Now that I am writing about it, I noticed a great effect on processing time (~10x) between the first working version I used for this report (only contains the union definition), compared to the current (contains the full RISC-V base I decoder as a function). Although the code added in the current version is not used by the top module test_union at all. The build process is visibly slower, each line in the output printout was taking longer. I suspect this could become a scalability issue for larger designs.

I questioned union support, since it was not implemented few months ago (I remember getting an explicit error stating non support) and because I got ERRORs and AST_STRUCT_ITEM lines with widths that did not match my code.

[tgorochowik]

Minimal test case that reproduces this issue (based on your original design):

// riscv_isa_pkg.sv
package riscv_isa_pkg;

typedef struct packed {
  logic [15:0] a;
  logic [15:0] b;
} op32_u_t;

typedef union packed {
  logic [31:0] xxx;
  op32_u_t u;
} op32_t;

// instruction decoder
function automatic logic dec32 (op32_t broken_arg);
endfunction

endpackage

// test_union.sv
package union_pkg;
endpackage
import riscv_isa_pkg::*;

module
test_union();
endmodule

// yosys command
read_systemverilog -top test_union -parse riscv_isa_pkg.sv test_union.sv

The culprit seems to be a combination of the package import and union as the function argument (function automatic logic dec32 (op32_t broken_arg);)

[tgorochowik]

Workaround:

diff --git a/systemverilog-plugin/UhdmAst.cc b/systemverilog-plugin/UhdmAst.cc
index b995107..f60b541 100644
--- a/systemverilog-plugin/UhdmAst.cc
+++ b/systemverilog-plugin/UhdmAst.cc
@@ -2426,7 +2426,7 @@ void UhdmAst::process_io_decl()

     visit_one_to_one({vpiTypedef}, obj_h, [&](AST::AstNode *node) {
         if (node) {
-            if (!node->str.empty()) {
+            if (!node->str.empty() && node->type != AST::AST_UNION) {
                 auto wiretype_node = new AST::AstNode(AST::AST_WIRETYPE);
                 wiretype_node->str = node->str;
                 // wiretype needs to be 1st node (if port have also another range nodes)

Apparently when a union is passed as a function argument, we don't correctly pass the ranges and they are striped to one bit (e.g. logic[15:0] a becomes logic a). This causes those weird error messages during simplification in yosys.

The workaround above treats all unions as anonymous when used as function arguments which forces it to copy the whole body.