-
Notifications
You must be signed in to change notification settings - Fork 404
[ImportVerilog] Add delayed assignment support #9085
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
Conversation
uenoku
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| } else if constexpr (std::is_same_v<OpTy, NonBlockingAssignOp>) { | ||
| delay = llhd::ConstantTimeOp::create( | ||
| rewriter, op->getLoc(), | ||
| llhd::TimeAttr::get(op->getContext(), 0U, "ns", 1, 0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it correct to assume 1ns delay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LLHD time values are a bit quirky (and in need of some rework). This creates a 0ns 1d 0e delay 🥲. I'll add a comment!
Originally, LLHD tried to explicitly model delta delays for VHDL interoperability, and it had an epsilon delay to accomodate the different scheduling regions of Verilog. But since moving everything to MLIR, I'm pretty sure we can make time values just be actual seconds, and deal with delta delays and the scheduling regions differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I misread` that :) Make sense!
| if (!implicitWaitOp) | ||
| return mlir::emitError(loc) << "implicit events cannot be used here"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Done!
Scheremo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice extension - LGTM, modulo @uenoku's comments.
Add the `moore.delayed_assign` and `moore.delayed_nonblocking_assign` ops to represent `assign #1ns a = b` and `a <= #1ns b` in SystemVerilog, respectively. Lower the new delay ops to `llhd.drv` with the appropriate delay value. Add support for blocking assignments with intra-assignment timing control to the ImportVerilog conversion. These are pretty trivial, since the blocking effects of the timing control are simply inserted in between computing the right-hand side and assigning to the left-hand side. Also add support for non-blocking assignments with intra-assignment delays. These require the new `moore.delayed_nonblocking_assign`, since the operation cannot suspend execution of the surrounding process. Instead, the assignment has to be added to the event queue and executed at a later point in time. Theoretically, the user could type wild things here, like `a <= repeat(5) @(posedge b or negedge c) d`, which would require us to spawn a separate "thread" to determine when all the events have occurred and the assignment can take place. We don't support any of that for now, because this is just utterly deranged. Also add support for delayed continuous assignments. This is pretty trivial since SystemVerilog only allows for simple delay values, such as `assign #1ns a = b`. This requires the new `moore.delayed_assign` op.
a9667d2 to
f8652f8
Compare
Add the
moore.delayed_assignandmoore.delayed_nonblocking_assignops to representassign #1ns a = banda <= #1ns bin SystemVerilog, respectively.Lower the new delay ops to
llhd.drvwith the appropriate delay value.Add support for blocking assignments with intra-assignment timing control to the ImportVerilog conversion. These are pretty trivial, since the blocking effects of the timing control are simply inserted in between computing the right-hand side and assigning to the left-hand side.
Also add support for non-blocking assignments with intra-assignment delays. These require the new
moore.delayed_nonblocking_assign, since the operation cannot suspend execution of the surrounding process. Instead, the assignment has to be added to the event queue and executed at a later point in time. Theoretically, the user could type wild things here, likea <= repeat(5) @(posedge b or negedge c) d, which would require us to spawn a separate "thread" to determine when all the events have occurred and the assignment can take place. We don't support any of that for now, because this is just utterly deranged.Also add support for delayed continuous assignments. This is pretty trivial since SystemVerilog only allows for simple delay values, such as
assign #1ns a = b. This requires the newmoore.delayed_assignop.