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

Internal Yosys nodes not removed #4738

Open
ashkanr65 opened this issue Nov 13, 2024 · 9 comments
Open

Internal Yosys nodes not removed #4738

ashkanr65 opened this issue Nov 13, 2024 · 9 comments
Labels
pending-verification This issue is pending verification and/or reproduction

Comments

@ashkanr65
Copy link

ashkanr65 commented Nov 13, 2024

Version

Yosys 0.47+46 (git sha1 9da7341, g++ 11.4.0-1ubuntu1~22.04 -fPIC -O3)

On which OS did this happen?

Linux

Reproduction Steps

I used custom library:

# Read in all Verilog files for the design
yosys read -sv "./designs/$::env(DESIGN)/verilog/*.v"

# Set the top module for the design hierarchy
yosys hierarchy -top $::env(DESIGN)

# Check for obvious problems in the current design (combinatorial loops, conflicts, etc.)
yosys prep;

# Extract and optimize finite state machines (FSMs)
yosys fsm; yosys opt;

# Translate memories to basic cells and optimize
yosys memory; yosys opt;

# Perform technology mapping
yosys techmap;

# Run the general synthesis process
yosys synth;

# Map D flip-flops to the specified library
yosys dfflibmap -liberty ./$::env(TECH)/SK_typ.lib;

# Perform logic optimization and mapping using ABC tool with the specified library
yosys abc -dff -liberty ./$::env(TECH)/SK_typ.lib;

# Perform functional reduction to simplify the design
yosys freduce;

# Map high-level logic to the specified cells -hicell LOGIC1 OUT
yosys hilomap -singleton -locell LOGIC0 OUT;

# Remove unused cells and wires with purge option
yosys opt_clean -purge;

# Perform final check on the design
yosys check;

# Write the synthesized Verilog output to the specified file
yosys write_verilog ./designs/$::env(DESIGN)/results/synth_$::env(DESIGN).v;

# extract the statistics of the design
yosys stat;

# Flatten the design hierarchy
yosys flatten;

Expected Behavior

Eliminate internal nodes

Actual Behavior

reg par3;
(* src = "./designs/qr_encoding_4x4_15_11/verilog/qr_encoding_4x4_15_11.v:26.9-26.13" *)
reg par4;

These internal nodes are appeared in the synthesized verilog code
So Openroad returns this error:
[ERROR STA-0164] ./designs/qr_encoding_4x4_15_11/results/run/synth_qr_encoding_4x4_15_11.v line 139, syntax error

@ashkanr65 ashkanr65 added the pending-verification This issue is pending verification and/or reproduction label Nov 13, 2024
@georgerennie
Copy link
Collaborator

If you enclose your script in three backticks (```) on the line before and after github won't try to expand it as markdown which is why it looks like you have big headers throughout the commands.

Without an actual testcase its hard to check exactly what behaviour you are seeing/expecting.

At a guess, this may be the same bug as #4712 which causes some processes not to be lowered correctly. Could you check if a build of Yosys with #4714 applied fixes it? If not, it would be helpful if you could provide a reproducible testcase (including inputs etc needed to reproduce this issue).

@ashkanr65
Copy link
Author

If you enclose your script in three backticks (```) on the line before and after github won't try to expand it as markdown which is why it looks like you have big headers throughout the commands.

Without an actual testcase its hard to check exactly what behaviour you are seeing/expecting.

At a guess, this may be the same bug as #4712 which causes some processes not to be lowered correctly. Could you check if a build of Yosys with #4714 applied fixes it? If not, it would be helpful if you could provide a reproducible testcase (including inputs etc needed to reproduce this issue).

Thank you for the quick reply. If you want I can share the test link with you. But I need to give me your email, then I can share the link with you.

@georgerennie
Copy link
Collaborator

Were you able to check if the pull request I linked fixes your issues? I think that would be a good starting point as this issue looks very similar to the issue that that pr fixes.

If you want I can share the test link with you. But I need to give me your email, then I can share the link with you.

I'm afraid that is probably more hassle than I am willing to go to for this, I don't have openroad installed and it is much easier for people looking at this issue later on if the reproducer can be attached to the github issue. If you are able to create a small reproducer that shows the same behaviour (perhaps start by trying to extract the definition of par3 and par4 from your sources) I can try to take a look at it, but I would start by checking if #4714 fixes your issues.

@ashkanr65
Copy link
Author

I updated to Yosys 0.47+56 (git sha1 a22ff47d6, clang++ 14.0.0-1ubuntu1.1 -fPIC -O3) But the error still there.
You don't need to have openroad. I can share the liberty file and verilog with you and you can test and see by yourself if you want.
It puts register block inside the synthesized code which openSTA cannot handle that.

@georgerennie
Copy link
Collaborator

georgerennie commented Nov 14, 2024

That PR is not yet merged into main so a22ff47 does not contain the patch.

If you have the github cli installed you can check it out by doing gh pr checkout 4714 or alternatively consider the instructions below (or from any guide on the internet explaining how to checkout a pr).

Assuming you have YosysHQ/yosys setup as a remote called origin (which is the default if you just git cloned this repo), you can checkout the patch by doing

# Fetch pr 4714 into new branch proc_dff_fix
git fetch origin pull/4714/head:proc_dff_fix

# Checkout new branch proc_dff_fix
git switch proc_dff_fix

@ashkanr65
Copy link
Author

Yosys 0.47+3 (git sha1 c23e64a23, clang++ 14.0.0-1ubuntu1.1 -fPIC -O3)
I suppose this is the one I think.
the registry blocks are still there:

  (* src = "./designs/qr_encoding_4x4_15_11/verilog/qr_encoding_4x4_15_11.v:25.9-25.13" *)
  reg par3;
  (* src = "./designs/qr_encoding_4x4_15_11/verilog/qr_encoding_4x4_15_11.v:26.9-26.13" *)
  reg par4;

@georgerennie
Copy link
Collaborator

Ahh that is frustrating, I suppose you can send it to georgerennie (at) gmail.com and I can try to have a look at it (or at least creating a reproducer that can be shared without giving away design details)

@ashkanr65
Copy link
Author

I sent you files.
Thank you.

@georgerennie
Copy link
Collaborator

The issue is with latches, which become $_DLATCH_P_ internally in this case, not being mapped to a latch from a liberty file. I'm afraid this is outside of my areas of knowledge with Yosys, I believe that dfflibmap can only map to DFFs from liberty files, not latches and that latches need separate techmapping files like in google/gf180mcu-pdk#101, but I am not certain as it isn't part of Yosys I know well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-verification This issue is pending verification and/or reproduction
Projects
None yet
Development

No branches or pull requests

2 participants