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

[Calyx-Firrtl] Fix undefined values bug by initializing to zero #1841

Merged
merged 2 commits into from
Jan 10, 2024

Conversation

ayakayorihiro
Copy link
Contributor

I ran into a problem with my FIRRTL backend where both the FIRRTL compiler and ESSENT would squash (for lack of a better term) guarded assignments into a straightforward assignments. For example, the FIRRTL code:

        done is invalid ; default initialization
        when wrapper_early_reset_static_seq_done.out:
            done <= UInt(1)

was compiled to the below Verilog:

assign done = 1'h1;

since there were no other assignments to done in the FIRRTL program. This caused the execution to finish in 0 cycles since done was always 1.

So, I fixed my backend to insert an assignment to 0, so it would produce the below FIRRTL:

        done is invalid ; default initialization
        done <= UInt(0)
        when wrapper_early_reset_static_seq_done.out:
            done <= UInt(1)

This solves the problem, as the FIRRTL compiler now produces the below Verilog:

assign done = wrapper_early_reset_static_seq_done_out;

and the intended output was produced.

Please let me know if I should fix anything/deal with things more elegantly! (I only changed 2 lines of Rust in this PR 😅 )

@ayakayorihiro ayakayorihiro self-assigned this Jan 9, 2024
@EclecticGriffin
Copy link
Collaborator

LGTM!

@ayakayorihiro ayakayorihiro merged commit 94dd5b0 into main Jan 10, 2024
7 checks passed
@ayakayorihiro ayakayorihiro deleted the firrtl-zero-initialization branch January 10, 2024 20:28
@sampsyo
Copy link
Contributor

sampsyo commented Jan 11, 2024

Amazing! Glad the fix was straightforward.

rachitnigam pushed a commit that referenced this pull request Feb 16, 2024
* Initializing to zero resolves the optimization issue we were seeing with language-tutorial-compute

* update tests to have zero initialization
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 this pull request may close these issues.

3 participants