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

level of non-const reset logged incorrectly? after #4569 #4869

Closed
donn opened this issue Jan 28, 2025 · 4 comments
Closed

level of non-const reset logged incorrectly? after #4569 #4869

donn opened this issue Jan 28, 2025 · 4 comments

Comments

@donn
Copy link
Contributor

donn commented Jan 28, 2025

Version

0.49

On which OS did this happen?

macOS

Reproduction Steps

mkdir and cd into an empty directory, where you download and untar this archive:

yosys_repro.tgz

Then /path/to/yosys run.ys.

Expected Behavior

For this negative asynchronous reset:

module non_const_reset(
    input clk,
    input rstn,
    input rstval,
    output reg value
);
    wire value_next = ~value;

    always_ff @ (posedge clk or negedge rstn)
        if (!rstn)
            value <= rstval;
        else
            value <= value_next;
endmodule

proc_dff (run as part of synth) should report the following:

3.2.9. Executing PROC_DFF pass (convert process syncs to FFs).
Creating register for signal `\non_const_reset.\value' using process `\non_const_reset.$proc$non_const_reset.v:9$2'.
Warning: Async reset value `\rstval' is not constant!
  created $aldff cell `$procdff$4' with positive edge clock and negative level non-const reset.

Actual Behavior

The issue bisects to #4569 (i.e. after Yosys 0.44+60)-- in the latest version, the log states the following:

It's worth mentioning that there was a separate issue with this patch when there are multiple asynchronous resets that was fixed in #4714.

3.2.9. Executing PROC_DFF pass (convert process syncs to FFs).
Creating register for signal `\non_const_reset.\value' using process `\non_const_reset.$proc$non_const_reset.v:9$2'.
Warning: Async reset value `\rstval' is not constant!
  created $aldff cell `$procdff$8' with positive edge clock and positive level non-const reset.

Notice how the non-const reset is reported as positive level.


Yet, with the provided mapping file, the result from synthesis is identical to the expected behavior. I've provided the logs AND output netlist from both a known-working version and 0.49 in the reproducible tarball.

To be 100% clear, I'm not sure if I'm misunderstanding something or if this is an actual logging bug.


EDIT: Updated the issue because I'd switched Expected and Actual Behavior sections around.

@donn donn added the pending-verification This issue is pending verification and/or reproduction label Jan 28, 2025
@povik
Copy link
Member

povik commented Jan 28, 2025

FYI @georgerennie @widlarizer

@widlarizer
Copy link
Collaborator

In both cases, the $aldff emitted by proc_dff itself has the same polarity. In the negedge case, there is an extra $not cell emitted. The log then correctly represents what happens inside Yosys, which as you show, leads to correct synthesis in the context of the whole flow. There might be more to this issue but I don't have the capacity to dig deeper at the moment

@widlarizer
Copy link
Collaborator

negedge.log
posedge.log

@widlarizer widlarizer removed the pending-verification This issue is pending verification and/or reproduction label Jan 28, 2025
@donn
Copy link
Contributor Author

donn commented Jan 28, 2025

Thanks for explaining!

@donn donn closed this as completed Jan 28, 2025
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

No branches or pull requests

3 participants