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

[ExportVerilog] Use wire for inlined op operand #5695

Merged
merged 3 commits into from
Jul 27, 2023

Conversation

seldridge
Copy link
Member

@seldridge seldridge commented Jul 26, 2023

Fix a bug where an assertion could fire in ExportVerilog due incomplete
wire creation in PrepareForEmission. This bug occurss if an always inline
operation was duplicated, but its operand is not always inline. This
creates a situation where, if you were to run PrepareForEmission again,
it would create a wire (and no assertion would fire).

Change the logic when recursing through operands of an always inline
operation to create this wire.

Fixes #5605.

This PR does some NFC monkeying with PrepareForEmission. The first two commits do this NFC monkeying. The third commit makes the change and adds tests. For the purposes of PR review, only the last commit matters.

Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Move a static function so that this can be used by an ealier static
function in a later commit.  This commit is broken out to make an actual
functional change obvious in a later commit.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Plumb a LoweringOptions const reference to more functions.  This is done
in preparation for a functional change in a later commit.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Fix a bug where an assertion could fire in ExportVerilog due incomplete
wire creation in PrepareForEmission.  This bug occurss if an always inline
operation was duplicated, but its operand is not always inline.  This
creates a situation where, if you were to run PrepareForEmission _again_,
it would create a wire (and no assertion would fire).

Change the logic when recursing through operands of an always inline
operation to create this wire.

Fixes #5605.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
@seldridge seldridge merged commit dbc043f into main Jul 27, 2023
5 checks passed
@seldridge seldridge deleted the dev/seldridge/issue-5605 branch July 27, 2023 14:05
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.

[ExportVerilog] Prepare for Emission Assertion Failure
3 participants