-
Notifications
You must be signed in to change notification settings - Fork 289
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
[Sim] Replace sim.func.dpi.call with sim.func.call and improve a verifier, NFC #7452
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -96,21 +96,21 @@ def DPIFuncOp : SimOp<"func.dpi", | |||||
}]; | ||||||
} | ||||||
|
||||||
def DPICallOp : SimOp<"func.dpi.call", | ||||||
def CallOp : SimOp<"func.call", | ||||||
[CallOpInterface, AttrSizedOperandSegments, | ||||||
DeclareOpInterfaceMethods<SymbolUserOpInterface>]> { | ||||||
let summary = "A call option for DPI function with an optional clock and enable"; | ||||||
let summary = "A call option for a function with an optional clock and enable"; | ||||||
let description = [{ | ||||||
`sim.func.dpi.call` represents SystemVerilog DPI function call. There are two | ||||||
optional operands `clock` and `enable`. | ||||||
`sim.func.call` represents a function call. Unlike a call operation in Func dialect, | ||||||
there are two optional operands `clock` and `enable`. | ||||||
|
||||||
If `clock` is not provided, the callee is invoked when input values are changed. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
If provided, the DPI function is called at clock's posedge. The result values behave | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
like registers and the DPI function is used as a state transfer function of them. | ||||||
|
||||||
`enable` operand is used to conditionally call the DPI since DPI call could be quite | ||||||
more expensive than native constructs. When `enable` is low, results of unclocked | ||||||
calls are undefined and in SV results they are lowered into `X`. Users are expected | ||||||
`enable` operand is used to conditionally call the function since function call | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
could be quite more expensive than native constructs. When `enable` is low, results | ||||||
of unclocked calls are undefined and in SV they are lowered into `X`. Users are expected | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume the same kind of 'undefined' as division by zero and out-of-bounds array accesses? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this belong to the same kind to div-by-zero. I intended undefined values definition in comb https://circt.llvm.org/docs/Dialects/Comb/RationaleComb/#undefined-values. I'll mention it in here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not really convinced that specifying how something is lowered to SV is a good way to specify an operation's semantics. Is it possible to define the semantics independent of SV in a way that allows this as a potential lowering? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's great point, thank you for suggestion! |
||||||
to gate result values by another `enable` to model a default value of results. | ||||||
|
||||||
For clocked calls, a low enable means that its register state transfer function is | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,5 +49,26 @@ hw.module.extern @non_func(out arg0: i1, in %arg1: i1, out arg2: i1) | |
|
||
hw.module @dpi_call(in %clock : !seq.clock, in %in: i1) { | ||
// expected-error @below {{callee must be 'sim.dpi.func' or 'func.func' but got 'hw.module.extern'}} | ||
%0, %1 = sim.func.dpi.call @non_func(%in) : (i1) -> (i1, i1) | ||
%0, %1 = sim.func.call @non_func(%in) : (i1) -> (i1, i1) | ||
} | ||
|
||
// ----- | ||
|
||
sim.func.dpi @mismatch(out arg0: i1, in %arg1: i2, out arg2: i1) | ||
|
||
hw.module @dpi_call(in %clock : !seq.clock, in %in: i1) { | ||
// expected-error @below {{'sim.func.call' op operand type mismatch: expected operand type 'i2', but provided 'i1' for operand number 0}} | ||
%0, %1 = sim.func.call @mismatch(%in) : (i1) -> (i1, i1) | ||
} | ||
|
||
|
||
// ----- | ||
|
||
// expected-note @below {{function result types: 'i1', 'i2'}} | ||
sim.func.dpi @mismatch(out arg0: i1, in %arg1: i1, out arg2: i2) | ||
|
||
hw.module @dpi_call(in %clock : !seq.clock, in %in: i1) { | ||
// expected-error @below {{'sim.func.call' op result type mismatch at index 1}} | ||
// expected-note @below {{op result types: 'i1', 'i1'}} | ||
%0, %1 = sim.func.call @mismatch(%in) : (i1) -> (i1, i1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "incorrect number of ..." error messages are not tested 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.