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

[SV] Mark sv.xmr.ref op as pure #6260

Merged
merged 4 commits into from
Oct 5, 2023
Merged

Conversation

fabianschuiki
Copy link
Contributor

@fabianschuiki fabianschuiki commented Oct 5, 2023

Having an sv.xmr.ref op inside a procedural block such as sv.alwayscomb triggers an assertion in PrepareForEmission. The pass would identify the ref op as having side-effects and then go ahead and try to pull it outside the procedural block. Doing so would create a sv.reg op with multiple nested inout types, which breaks.

Fix the issue by marking the sv.xmr.ref op as pure. Taking a reference to something does not have a side-effect. It's accessing what's behind the reference that has side-effects.

Having an `sv.xmr.ref` ops inside a procedural block such as
`sv.alwayscomb` triggers an assertion in `PrepareForEmission`. The pass
would identify the ref op as having side-effects and then go ahead and
try to pull it outside the procedural block. Doing so would create a
`sv.reg` op with multiple nested inout types, which breaks.

Fix the issue by marking the `sv.xmr.ref` op as pure. Taking a reference
to something does not have a side-effect. It's accessing what's behind
the reference that has side-effects.
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

LGTM

def XMRRefOp : SVOp<"xmr.ref", [
DeclareOpInterfaceMethods<SymbolUserOpInterface>,
Pure
]> {
Copy link
Member

Choose a reason for hiding this comment

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

Nice indentation of traits!

Copy link
Contributor

@dtzSiFive dtzSiFive left a comment

Choose a reason for hiding this comment

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

I think this makes sense, thanks!

@fabianschuiki fabianschuiki merged commit 71a78d6 into main Oct 5, 2023
5 checks passed
@fabianschuiki fabianschuiki deleted the fschuiki/make-sv-xmr-ref-pure branch October 5, 2023 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Verilog/SystemVerilog Involving a Verilog dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants