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

GateMate SystemVerilog support #3

Closed
TarikHamedovic opened this issue May 21, 2024 · 28 comments
Closed

GateMate SystemVerilog support #3

TarikHamedovic opened this issue May 21, 2024 · 28 comments
Assignees
Labels
enhancement New feature or request

Comments

@TarikHamedovic
Copy link
Collaborator

Using the line:
logic [PIPELINE_DELAY:1] [BANK_WIDTH-1:0] bankb_p ;
Gives an error:
ERROR: syntax error, unexpected '[', expecting TOK_ID or '#'

The error can be fixed using an unpacked array, but sometimes we need a packed array.

@chili-chips-ba
Copy link
Owner

chili-chips-ba commented Jun 3, 2024

Insight from Patrick Urban of Cologne Chip (@pu-cc) : "... from what I have read, you are also trying to implement this on ulx3s with lattice ecp5, right? Are you also using Yosys? You should have noticed this too, as this is not a GateMate-related issue but is due to incomplete SystemVerilog support in Yosys. Another approach is to use sv2v. We had customers who got on well with it. https://github.com/zachjs/sv2v ..."

@chili-chips-ba
Copy link
Owner

We understand that as-is Yosys comes with very rudimentary support for SV. However, given that (contrary to LatticeSemi), Yosys is the only synthesis tool for GateMate, would it not make more sense for CologneChip to prepackage their version of Yosys with https://github.com/chipsalliance/synlig plugin for everyone?!

While we can also do that, and are doing it, rather than forcing all users into repeating this same work, it seems more beneficial for CologneChip to do it once for all, by making it the standard part of toolkit and install instructions, which currently don't mention it at all:
https://www.colognechip.com/docs/ug1002-toolchain-install-latest.pdf

@chili-chips-ba
Copy link
Owner

chili-chips-ba commented Jun 8, 2024

Given that the entire Synlig Yosys story is confusing, it is even more important for CologneChip to properly package it and validate at their very source.

The problems are multi-fold:

  1. Synlig plugin comes pre-packaged with Yosys, but that Yosys is not only more than 2 years old, but also not configured for GateMate
  2. Synlig plugin can also be installed standalone (without Yosys that cames with it) and paired up with latest Yosys, but such combo simply does not work
  3. Yosys is known for its ups and downs, i.e. certain level of "moodiness", hence needs supervision when it comes to external plugins
  4. Since only certain combinations work, they should be provided and validated by the FPGA vendor. Our examples can serve as part of that validation package.

@cliffordwolf for her first-hand Yosys insights on this topic.
@kamilrakoczy for advice on Synlig and Yosys compatibility, or not.

@chili-chips-ba chili-chips-ba changed the title Error when running synthesis for SystemVerilog file with a packed array structure GateMate SystemVerilog support Jun 8, 2024
@gtaylormb
Copy link
Collaborator

Using the line: logic [PIPELINE_DELAY:1] [BANK_WIDTH-1:0] bankb_p ; Gives an error: ERROR: syntax error, unexpected '[', expecting TOK_ID or '#'

The error can be fixed using an unpacked array, but sometimes we need a packed array.

This syntax appears to be supported in Yosys 0.42.

@chili-chips-ba
Copy link
Owner

chili-chips-ba commented Jun 9, 2024

@gtaylormb have you been able to synthesize your entire OPL3 RTL with this new, fresh-from-the-press Yosys 0.42 ?

image

@gtaylormb
Copy link
Collaborator

Nope, it still doesn't like package imports :(

@chili-chips-ba
Copy link
Owner

Any luck with package imports when Synlig plugin is also in the play? @tgorochowik

@gtaylormb
Copy link
Collaborator

No, doesn't look like it unfortunately.

@chili-chips-ba
Copy link
Owner

@gtaylormb
Copy link
Collaborator

Eh, looks like the Makefile is pretty non-functional.

synth: synth_vlog

This was calling the Verilog Makefile target. Imports potentially working, running into some other error saying it can't find the top level module, opl3. Anyway I gotta get to sleep but good luck

@chili-chips-ba
Copy link
Owner

@zachjs @hzeller how do you feel about adding this super-cool SV design to your SV2V regression suite? The idea is to enable synthesizing it with Yosys, which is apparently going belly up on SV even with Synlig plugin.

@gtaylormb
Copy link
Collaborator

Had a chance to dig a little more into this. Indeed the import issue was because the default Makefile target synth chooses the Verilog target instead of the SystemVerilog target. If I keep my Yosys path to the default (which is the old version bundled with the toolchain, 0.36), I get all those disturbing warnings as described here: chipsalliance/synlig#2433

If I update the Yosys path to the latest version (0.42) that I compiled myself I get the missing top level module opl3 error. Perhaps the Synlig plugin is version locked to Yosys version that's bundled, but in any case it doesn't get far at all. I separately verified that Yosys 0.42 supports multidimensional packed arrays by modifying one of their test cases. Passes in 0.42, fails in 0.36.

Anyway, it appears SystemVerilog support in Yosys/Synlig has a long way to go based on those warnings.

@hzeller
Copy link

hzeller commented Jun 9, 2024

Yeah, synlig and yosys are somewhat version locked as synlig uses some yosys APIs. Unfortunately, some sponsors' priorities shifted, so updates in synlig, including following the latest Yosys, are somewhat slow currently. So pull requests on synlig to get it to work with latest Yosys are welcome. There was an attempt apparently but if fizzled out chipsalliance/synlig#2431

I see what I can do to get opl3_fpga added to the sv-test suite.

For now, I've added it to the smoke-tests in the syntax-checkker/formatter/language-server Verible CI.

@chili-chips-ba
Copy link
Owner

chili-chips-ba commented Jun 9, 2024

... is it then fair to say that the main part of the problem comes from the fact that the latest synlig

image

comes packaged with older yosys? And, if we were to compile the latest yosys separately, it would simply not work with the latest synlig due to some API discrepancies?!

Great that adding OPL3 design to the synlig SV test suite is under consideration. Is the same suite of tests also used for SV2V validation?

  • @TarikHamedovic, have you had chance to try passing OPL3 RTL through SV2V flow?

BTW, we love Verible -- It's a super-cool tool!

@hzeller
Copy link

hzeller commented Jun 9, 2024

Yes, currently the thing with synlig is that there is some API changes in Yosys which does not make it compile anymore with the latest Yosys. Getting that fixed up would be great.

At least sv2v is part of the sv-tests suite, but Zach probably has other tests as well. (sv-tests is independent of the various parsers, it just does some tests on a bunch of parsers, but that is independent of the tests that the projects do).

@chili-chips-ba
Copy link
Owner

@hzeller Re: "sv-test... independent of the tests that the projects do..." and "... Zach probably has other tests as well..."

It seems that the ChipsAlliance runs and publishes their own assessment of the level of each tool's SV support. Yosys is apparently also subject to this independent, 3rd-party compliance scrutiny.

Based on the sv-test results and what you know about the overall ecosystem, which path would you recommend for opensource synthesis of SV RTL?

While @TarikHamedovic is soon to report the results of his opl3_fpga experiments with SV2V, if both Synlig and SV2V end up as disappointments, is there another path to try?

@QuantamHD
Copy link

I won't discourage anyone from pursing any technical end, but synlig has a number of things working against it. Namely, the solution is split between 4 repos (Surelog, Synlig, UDHM, Yosys) all of which need to be in perfect sync to make things work. Synlig also relies heavily on private Yosys APIs which the upstream Yosys team is under no obligation to support.

If it were up to me I would look more heavily into https://github.com/povik/yosys-slang which relies on the slang parser, and is an overall less complicated solution.

@chili-chips-ba
Copy link
Owner

Thank you, great tip, we are in the process of trying opl3_fpga with both yosys-slang and sv2v.

  • Any other recommendation for trying now that we have the momentum?!

@zachjs
Copy link

zachjs commented Jun 9, 2024

I'm of course eager for you to give sv2v a shot, and welcome any bug reports, feature requests, or contributions. Indeed sv2v has hundreds of its own tests, generally focusing on just a few features at a time. I do use the sv-tests dashboard from time to time to check in on its assessment of sv2v.

@chili-chips-ba
Copy link
Owner

... based on @TarikHamedovic and @tarik-ibrahimovic thus-far testing, the SV2V is more mature than Yosys-Slang, and more stable than Synlig.

  • Going forward, we are focusing on the SV2V, i.e. investing more time and effort into it.

@zachjs, may we suggest to add both OPL3_FPGA and our openCam-SI SystemVerilog RTL to your personal SV2V test suite.

@hzeller please consider adding the latter to both Verible smoke-tests and sv-tests ...

@phsauter
Copy link

We also had a good amount of problems dealing with our (PULP) SystemVerilog, especially around properly resolving parameters.

For this reason we made SVase it is also based on Slang, just like Yosys-Slang but we only use it as a source-to-source pre-elaborator.
Specifically we propagate all parameters to the scope where they are used and we uniquify all unique parametrizations of modules and unroll all generate statements.
If you run into problems around parameters, you might want to give it a try.

@chili-chips-ba
Copy link
Owner

chili-chips-ba commented Jun 15, 2024

While both SV2V and SVase preprocessing steps can be incorporated into our build flow, there is still advantage in the organic integration of these two preprocessors.

  • Is any thought or effort expended in that direction?!

@phsauter
Copy link

You are right and yes!

I can't really say more at this point than this: we are currently formulating what exactly we want from a SystemVerilog frontend, likely based on Slang.

@chili-chips-ba
Copy link
Owner

chili-chips-ba commented Jun 15, 2024

... it sounds like you are considering to integrate SVase with Slang, whereas SV2V is more popular and overall more mature. As you've mentioned, both PULP and lowRISC are using SV2V.

If this work on formulating your plans is in the public domain, please share link to the relevant discussion forum...

@phsauter
Copy link

SVase uses Slang as its parser library (ie it is built on-top of Slang).

What we are currently looking into is to build a Yosys frontend that directly uses Slang as a library.
It is not yet in the public domain so I can't share anything else.

For our current projects like Basilisk we actually use Morty (our own tool) to collect files and pickle them into one single file (this is not super necessary but makes it easier to exchange the RTL with others). Then we use SVase to resolve the sometimes rather complex parameters we have and finally SV2V for the bulk of the code.

@chili-chips-ba
Copy link
Owner

... given @gtaylormb thumbs up for Slang Linter, if anyone knows of comparisons with Verible, please share links...

@chili-chips-ba
Copy link
Owner

@TarikHamedovic if, with @zachjs SV2V now firmly entrenched in our flow, you believe that the original question is answered and discussion topic exhausted, please consider closing this issue.

@TarikHamedovic
Copy link
Collaborator Author

After trying out different available solutions, sv2v proved to be the best option for now, the synthesis of the generated Verilog files works and I consider that the original topic of the issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants