Skip to content

[ImportVerilog] Implement procedural continuous assignment#9795

Open
Arya-Golkari wants to merge 2 commits intollvm:mainfrom
Arya-Golkari:procedural_continuous_assignment
Open

[ImportVerilog] Implement procedural continuous assignment#9795
Arya-Golkari wants to merge 2 commits intollvm:mainfrom
Arya-Golkari:procedural_continuous_assignment

Conversation

@Arya-Golkari
Copy link
Contributor

This PR supports lowering procedural continuous assignment statements (i.e. using the assign, deassign, force, and release keywords) into Moore.

I'm not very confident in my work, so I'd really appreciate help from reviewers! @Scheremo @fabianschuiki

@fabianschuiki
Copy link
Contributor

Hey @Arya-Golkari, thanks for taking a stab at this! I'm wondering if we should create dedicated Moore dialect ops for these procedural assignments. They are pretty weird: if a procedure contains an assign ...; statement, that assignment behaves like a non-procedural/continuous assignment directly in the moore.module, but with an enable condition that is controlled procedurally. Maybe something like this?

module Foo(input int y);
  int x;
  initial begin
    #1;
    assign x = 42;
    #1;
    assign x = y;
    #1;
    deassign x;
    #1;
  end
endmodule

I would expect this to have x not driven in the first time step, then driven to 42 in the second time step, then continuously driven to whatever value y is in the third timestep, and then not driven at all again in the fourth timestep. We might be able to emit this by separating the procedural part that decides if and what to drive, from the actual assignment itself:

moore.module @Foo(in %y: i32) {
  %x = moore.variable : i32
  %proc_assign_enable = moore.variable 0 : i1
  %proc_assign_value = moore.variable 0 : i32
  moore.cond_assign %x, read(%proc_assign_value), read(%proc_assign_enable)
  moore.procedure initial {
    // wait
    moore.blocking_assign %proc_assign_value, 42
    moore.blocking_assign %proc_assign_enable, 1
    // wait
    // I don't remember if semantics are to assign the value y has at the time of the
    // procedural assignment, or if any changes in y while the assignment is active
    // should propagate to x. If the latter is the case, we'll need a multiplexer in the
    // module which we can switch over to continuously read y and feed it into the
    // cond_assign.
    moore.blocking_assign %proc_assign_value, read(%y)
    moore.blocking_assign %proc_assign_enable, 1
    // wait
    moore.blocking_assign %proc_assign_enable, 0
    // wait
  }
}

If we create dedicated procedural assign/deassign ops, we'll have to add a transformation pass that pretty much immediately removes them again and turns them roughly into the above. WDYT?

Copy link
Contributor

@TaoBi22 TaoBi22 left a comment

Choose a reason for hiding this comment

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

Looks good, just a few docs comments!

LHS of assignment is a variable reference, a net, a constant bit-select or
part-select of a vector net, or a concatenation of these.

It overrides an `assign` procedural continuous assignment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: could we move this up to the first sentence to make the semantic difference a bit clearer?


If the variable is driven by a continuous assignment or currently has an active
`assign` procedural continuous assignment, then it shall re-establish that
assignment. Otherwise,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there's some missing text here?

]> {
let summary = "Procedural continuous deassignment (keyword: release)";
let description = [{
Deassignment of a continuous assignment in procedure scope, such as `release x;`.
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, could we clarify the semantic difference between this and deassign in this paragraph?

Comment on lines +1122 to +1126
if (deassignNode.isRelease) {
moore::ProceduralContinuousReleaseOp::create(builder, loc, lhs);
} else {
moore::ProceduralContinuousDeassignOp::create(builder, loc, lhs);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we can drop the braces here

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