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

Synthesis produces incorrect logic (erroneously optimized to 0) #2824

Closed
smunaut opened this issue Jun 11, 2021 · 1 comment · Fixed by #2825
Closed

Synthesis produces incorrect logic (erroneously optimized to 0) #2824

smunaut opened this issue Jun 11, 2021 · 1 comment · Fixed by #2825

Comments

@smunaut
Copy link
Contributor

smunaut commented Jun 11, 2021

The attached file contains what's needed to reproduce the issue :

synth_bug.tar.gz

It was tested on latest oss-cad-suite build from 20210610 ( Yosys 0.9+4081 git sha1 1667ad6 ).

In the above package, the makefile will synthesize two different top level, wrapping the same module.
In one case the same signal is fed to both the submodule input and in the other case they are different.
The submodule logic is basically a AND gate, but written using a case (It's from the picorv32 sources, with as many things trimmed as possible to still show the bug).

When the two signals are separate, I expect an AND gate to be generated and that's the case. That's what in synth.good.v.
In the other case, it'd be the AND of 2 identicals signals and so this should be just a wire. And this is NOT the case ... looking in synth.bad.v you find assign pcpi_int_wr = 1'h0; which is obviously wrong.

This test case was narrowed down from the Hackaday Conference FPGA Badge from 2019. The bitstream would build fine with the 2019 toolchain but would fail (it builds but the CPU misbehaves) with recent toolchain. The commit where it started breaking is the new opt dff pass merged in August 2020, however this is not what's actually causing the issue. This change just exposed it. The new DFF opt is just better at optimizing and instead of having 2 distinct FFs outputing to pcpi_wr and pcpi_ready, it realized it could use the same FF and this triggered this bug. The final result in the SoC is that pcpi_int_wr is stuck to 0, causing latched_write to be 0 for PCPI instruction causing issues ... (this was confirmed in simulation and also on real hw with a logic analyzer looking at those signals).

@smunaut
Copy link
Contributor Author

smunaut commented Jun 11, 2021

Looking at intermediate steps, looks like opt_muxtree is where it goes wrong.

read_verilog mini_dut.v
hierarchy -top mini_dut_wrap_bad
flatten
proc
opt_expr
write_verilog -norename good.v
opt_muxtree
write_verilog -norename bad.v

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 a pull request may close this issue.

1 participant