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 feature to select Yosys version #2965

Closed
dkongg opened this issue Nov 5, 2024 · 11 comments · Fixed by #3056
Closed

Add feature to select Yosys version #2965

dkongg opened this issue Nov 5, 2024 · 11 comments · Fixed by #3056
Labels
enhancement New feature or request

Comments

@dkongg
Copy link

dkongg commented Nov 5, 2024

Feature Description

Hello,
I have some RTL code that passes through the flow in Yosys 0.44. It seems like the Yosys version has been updated to 0.46.
Now floorplanning fails with the following error: 12: [ERROR STA-0164] inputs/design.vg line 6, syntax error, unexpected REG.
However, floorplanning didn't fail when Yosys 0.44 was used for synthesis.

@dkongg dkongg added the enhancement New feature or request label Nov 5, 2024
@gadfort
Copy link
Contributor

gadfort commented Nov 5, 2024

@dkongg you can always just install the needed version of yosys, we only compile and test against a single version.
It sounds like what you are really encountering is an issue with openroad/opensta. It might be helpful to have the testcase (if it can be shared) or just the anonymised lines from inputs/design.vg sounds like the first 7 lines would be sufficient.

@dkongg
Copy link
Author

dkongg commented Nov 5, 2024

@gadfort here are the first few lines of the .vg file. I think what's happening is that the tool is unable to implement when the first row inside the module is a reg instead of a wire.

`/* Generated by Yosys 0.46 (git sha1 e97731b9d, g++ 9.4.0-1ubuntu1~20.04.2 -fPIC -O3) /
(
top = 1 )
(
src = "inputs/design.v:82.1-172.10" )
module design(clk, rst, en, plaintext, key, ciphertext, done);
reg $auto$verilog_backend.cc:2352:dump_module$58570 = 0;
(
src = "inputs/design.v:0.0-0.0" )
wire [31:0] 00000;
(
src = "inputs/design.v:0.0-0.0" )
wire [31:0] 00001;
(
src = "inputs/design.v:0.0-0.0" *)

@gadfort
Copy link
Contributor

gadfort commented Nov 5, 2024

how was this generated? the reg $auto$verilog_backend.cc:2352:dump_module$58570 = 0; is highly unusual.
This seems like yosys failed to do something, since the output of yosys should only contain gates and wires, it cannot have registers since that should have been converted to a FF.
I assume this is not driving anything in the design?

@dkongg
Copy link
Author

dkongg commented Nov 6, 2024

I ran this remotely on the cloud server and the job ID is 329f111e73f348f39e86c1b5d1f9ea55.
There are no synthesis errors but there is a synthesis warning saying:
"Use "proc" to convert processes to logic networks and registers."
Interestingly, the full flow was working when yosys 0.44 was used. Perhaps the "proc" command could be added in the synthesis script?

@gadfort
Copy link
Contributor

gadfort commented Nov 6, 2024

@dkongg it looks like yosys broke something, proc is already included, but looking at the logs it seems to fail on something.
Is the code sharable? If yes, I would like to generate a testcase for the yosys team so they can fix what broke.

@dkongg
Copy link
Author

dkongg commented Nov 6, 2024

The code is shareable, thank you.

@gadfort
Copy link
Contributor

gadfort commented Nov 6, 2024

I've opened an issue with yosys.
Looking at the log you also have this:

Warning: Async reset value `\key [31:0]' is not constant!
  created $aldff cell `$procdff$1503' with positive edge clock and positive level non-const reset.

which appears to unmapped also in the 0.44 version.

@gadfort
Copy link
Contributor

gadfort commented Nov 7, 2024

This fixes the issue: YosysHQ/yosys#4714 you still have the $_ALDFFE_PPP_ in the design which are not mapped correctly, but that issue appears to have been present since 0.44

@gadfort gadfort linked a pull request Dec 11, 2024 that will close this issue
@dkongg
Copy link
Author

dkongg commented Dec 11, 2024

Hi @gadfort, would it be possible to update the Yosys version to one that been fixed on the remote server? (seems like the main branch on Yosys doesn't have the fix... YosysHQ/yosys#4712 (comment))

@gadfort
Copy link
Contributor

gadfort commented Dec 11, 2024

@dkongg the remote will update once I make a new release of SC. My hope is that will be today at some point. The version of yosys (yosys 0.48) they released today contains the fix (YosysHQ/yosys#4714).

@dkongg
Copy link
Author

dkongg commented Dec 11, 2024

thanks!

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

Successfully merging a pull request may close this issue.

2 participants