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

[HW][SV] Allow procedural SV ops in hw::TriggeredOp #7314

Closed
wants to merge 1 commit into from

Conversation

fzi-hielscher
Copy link
Contributor

Adapt the SV dialect ProceduralOp / NonProceduralOp traits to consider hw::TriggeredOp a procedural region. This should enable us to lower the procedural operations within a hw::TriggeredOp region before lowering the entire module.

See the discussion in #7292. We currently do not have a cross-dialect notion of procedural regions. However, the description of hw::TriggeredOp states bluntly that it creates a procedural region, so I think this is a relatively intuitive change.

Piggybacked are two sanity checks for hw::TriggeredOp:

  • Don't allow nesting of TriggeredOps
  • Ensure the number of input arguments matches the number of block arguments

@fzi-hielscher fzi-hielscher added HW Involving the `hw` dialect Verilog/SystemVerilog Involving a Verilog dialect labels Jul 12, 2024
@fzi-hielscher fzi-hielscher marked this pull request as ready for review July 12, 2024 12:57
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.

Hmm, I'm not sure it's reasonable change to regard regions hw::TriggeredOp as ProceduralRegion in verifier. I'm more comfortable if hw::TriggeredOp just implements ProceduralRegion trait in that case. In order to do that I think it's necessary to promote the notion of procedural regions from SV to HW but it will require more broader discussion.

@fzi-hielscher
Copy link
Contributor Author

I agree that this is far from ideal. I had, in fact, tried to move ProceduralRegion into HW, but then stumbled over the question on how to deal with this transitively w.r.t. upstream dialects (notably SCF). CC #719.

So, I went with the quick and dirty solution allowing me to temporarily have a sv.if in a hw.triggered. 😬

@fzi-hielscher
Copy link
Contributor Author

Superseded by #7335 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HW Involving the `hw` dialect Verilog/SystemVerilog Involving a Verilog dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants