Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to use systemverilog-plugin to build examples #244

Merged
merged 5 commits into from
Apr 5, 2022

Conversation

kamilrakoczy
Copy link
Contributor

@kamilrakoczy kamilrakoczy commented Jan 13, 2022

This PR adds option to build examples using systemverilog (renamed uhdm) plugin.
Not all examples are working yet with it, so this PR enables systemverilog only as option, not as default frontend.

Progress on supporting currently not working examples can be tracked here: chipsalliance/yosys-f4pga-plugins#287

Signed-off-by: Kamil Rakoczy krakoczy@antmicro.com

@nelsobe
Copy link
Contributor

nelsobe commented Jan 21, 2022

@kamilrakoczy I see that this fails a bunch of tests but does pass a number of xc7 tests. We would like to use it on xc7 on basic designs. Is it functional enough for us to try? If not, will it be soon?

I assume if we were to try it we would do the manual copying of the symbiflow_synth script?

@mithro
Copy link
Contributor

mithro commented Jan 23, 2022

@kgugala / @hzeller - Can you help keep this moving forward?

@kamilrakoczy
Copy link
Contributor Author

@nelsobe
I've tested some of the xc7 designs on the hardware (Arty A35T) and at least counter_test and pulse_width_led are working out of the box. Some of the designs are passing yosys, but are failing on SDC plugin, as it can't find net with given name. I suspect that if this name would be adjusted to the name generated with uhdm-plugin, this designs can also work.
I think it is already in the state that is functional enough to try basic designs.

Yes, until f4pga/f4pga-arch-defs#2364 will be merged, manual copying of symbiflow_syth is required.

@nelsobe
Copy link
Contributor

nelsobe commented Feb 4, 2022

@kamilrakoczy Thanks for the info! We have successfully run it on a simple design without errors (hardware test successful as well). I am thinking it would be easy (given how we are collecting student designs) to run all the student designs through both paths.

Any thoughts on the net naming problem? Happy to have a student take a look, do you have any pointers on where to point him?

@kamilrakoczy
Copy link
Contributor Author

kamilrakoczy commented Feb 7, 2022

@nelsobe I briefly looked at picosoc_demo example that fails with missing clk_bufg net.

Adding -debug flag to Surelog commands allows to print more debug information from Surelog/yosys, e.g.:

TARGET="arty_35" SURELOG_CMD="-parse -DSYNTHESIS -debug" make -C picosoc_demo

I confirmed that clk_bufg is correctly read by Surelog and then passed as AST_WIRE to yosys, but it is missing from top_synth.v that is flattened verilog after some initial yosys passes.
I'd say that first step would be to find out why this wire is removed by yosys, so we would need to identify the pass that removes this wire and then we can analyze the code to find out the reason.

As it looks like the problem is in uhdm-plugin, in case of questions regarding implementation details of uhdm-plugin (or any other issues) feel free to open issue in: https://github.com/antmicro/yosys-uhdm-plugin-integration/issues.

Please let us know if you need any additional guidance.

GitHub
Contribute to antmicro/yosys-uhdm-plugin-integration development by creating an account on GitHub.

@kamilrakoczy
Copy link
Contributor Author

@nelsobe PR in symbiflow-arch-def adding native support for uhdm-plugin to symbiflow toolchain got merged.
Now it is not needed to manually copy symbiflow_synth script. I've updated this PR to reflect this change.

@DCrom1
Copy link

DCrom1 commented Feb 14, 2022

@kamilrakoczy I have generated a few test designs and can see that they all exhibit the problem when the top level ports are multi-bit signals. This seems to be for inputs only. Been talking with Prof Nelson @nelsobe, not sure where to look next since I don't know for sure what the structure of surelog/uhdm/yosys is. Can you provide some ideas on where to look in the code to identify where the subscripts are being lost for inputs? Happy to continue to dig but am a bit lost as to where the code to be looking in might be found.

@kamilrakoczy
Copy link
Contributor Author

kamilrakoczy commented Feb 15, 2022

Hello, @DCrom1

Source code of the used tools can be found in following repos:
Surelog: https://github.com/chipsalliance/Surelog
uhdm-plugin: https://github.com/SymbiFlow/yosys-symbiflow-plugins/tree/master/uhdm-plugin
yosys: https://github.com/yosysHQ/yosys

When design is read using read_verilog_with_uhdm (that is internally called by symbiflow_synth: https://github.com/SymbiFlow/symbiflow-arch-defs/blob/master/xc/xc7/toolchain_wrappers/symbiflow_synth#L127) we are executing UhdmSurelogAstFrontend that is defined in uhdm-plugin: https://github.com/SymbiFlow/yosys-symbiflow-plugins/blob/master/uhdm-plugin/uhdmsurelogastfrontend.cc#L79.
This frontend is internally setting up Surelog and converts design to (in-memory) UHDM file: https://github.com/SymbiFlow/yosys-symbiflow-plugins/blob/master/uhdm-plugin/uhdmsurelogastfrontend.cc#L148.
We are parsing this UHDM file and convers nodes to Yosys AST: https://github.com/SymbiFlow/yosys-symbiflow-plugins/blob/master/uhdm-plugin/UhdmAst.cc#L3908.
When we have Yosys AST we pass it to yosys for simplification and other passes (just like it was read using read_verilog command): https://github.com/SymbiFlow/yosys-symbiflow-plugins/blob/master/uhdm-plugin/uhdmsurelogastfrontend.cc#L157

As picosoc_demo design is working fine using regular read_verilog, I think the problem is in Surelog or uhdm-plugin.

I suggest that first you should confirm, that this multi-bit signal information is present in UHDM file. To do that, please add -debug to SURELOG_CMD and search for given signal name.

Example SystemVerilog code:

input pkg::anon_struct_typedef [3:0] port_packed

Example UHDM output (please look under uhdmtopModules, uhdmallModules doesn't contain all information about signal). UHDM structure is (almost, with few exceptions) identical to VPI from standard:

...
|uhdmtopModules:
...
    |vpiNet:
    \_packed_array_net: (work@anon_module.port_packed), line:9:38, endln:9:49, parent:work@anon_module
      |vpiName:port_packed
      |vpiFullName:work@anon_module.port_packed
      |vpiRange:
      \_range: , line:9:33, endln:9:36
        |vpiLeftRange:
        \_constant: , line:9:33, endln:9:34
          |vpiDecompile:3
          |vpiSize:64
          |UINT:3
          |vpiConstType:9
        |vpiRightRange:
        \_constant: , line:9:35, endln:9:36
          |vpiDecompile:0
          |vpiSize:64
          |UINT:0
          |vpiConstType:9
      |vpiElement:
      \_struct_net: 
        |vpiTypespec:
        \_struct_typespec: (pkg::anon_struct_typedef), line:2:12, endln:2:18
...

If it is present, next step would be to confirm, that it is present in Yosys AST (before simplification, after uhdm-plugin). This information is also printed after adding -debug.

Example Yosys output:

Dumping AST before simplification:
...
       AST_WIRE   str='\port_packed' input logic basic_prep range=[49:0] multirange=[ 0 50 0 4 ] multirange_swapped=[ 0 0 ]
         ATTR \wiretype:
           AST_CONSTANT   str='\anon_struct_typedef' bits='0101110001100001011011100110111101101110010111110111001101110100011100100111010101100011011101000101111101110100011110010111000001100101011001000110010101100110'(160) basic_prep range=[159:0] int=1701078374
         ATTR \packed_ranges:
           AST_CONSTANT   bits='1'(1) range=[0:0] int=1
             AST_RANGE   basic_prep range=[49:0]
               AST_CONSTANT   bits='00000000000000000000000000110001'(32) signed basic_prep range=[31:0] int=49
               AST_CONSTANT   bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0]
             AST_RANGE   basic_prep range=[3:0]
               AST_CONSTANT   bits='00000000000000000000000000000011'(32) signed basic_prep range=[31:0] int=3
               AST_CONSTANT   bits='00000000000000000000000000000000'(32) signed basic_prep range=[31:0]
         ATTR \unpacked_ranges:
           AST_CONSTANT   bits='1'(1) range=[0:0] int=1
         AST_RANGE   range=[199:0]
           AST_CONSTANT   bits='00000000000000000000000011000111'(32) signed range=[31:0] int=199
           AST_CONSTANT   bits='00000000000000000000000000000000'(32) signed range=[31:0]
...

If you are not sure, how correct AST should look like, you can try to compare AST of given node between when it is read using read_verilog_with_uhdm and read_verilog. Node read using uhdm-plugin can differ (e.g. have additional attributes), but generally they should look similar.

@rkapuscik rkapuscik force-pushed the uhdm_plugin branch 2 times, most recently from a008848 to 3172b03 Compare March 23, 2022 13:39
@rkapuscik rkapuscik force-pushed the uhdm_plugin branch 5 times, most recently from 8aff40f to ba28cab Compare April 4, 2022 14:06
@kamilrakoczy kamilrakoczy force-pushed the uhdm_plugin branch 4 times, most recently from 7a7661e to 4093eb1 Compare April 5, 2022 09:18
Signed-off-by: Kamil Rakoczy <krakoczy@antmicro.com>
Signed-off-by: Kamil Rakoczy <krakoczy@antmicro.com>
@kamilrakoczy kamilrakoczy changed the title WIP: Use uhdm-plugin to build examples Add option to use systemverilog-plugin to build examples Apr 5, 2022
@kamilrakoczy kamilrakoczy marked this pull request as ready for review April 5, 2022 09:33
Signed-off-by: Rafal Kapuscik <rkapuscik@antmicro.com>
Signed-off-by: Kamil Rakoczy <krakoczy@antmicro.com>
Signed-off-by: Kamil Rakoczy <krakoczy@antmicro.com>
Copy link
Collaborator

@mkurc-ant mkurc-ant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's wait for CI.

@mkurc-ant mkurc-ant merged commit c8dab8d into chipsalliance:master Apr 5, 2022
@kamilrakoczy kamilrakoczy deleted the uhdm_plugin branch April 5, 2022 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants