diff --git a/include/circt/Dialect/Sim/SimOps.td b/include/circt/Dialect/Sim/SimOps.td index c3bdb370f23b..6e5708a0e831 100644 --- a/include/circt/Dialect/Sim/SimOps.td +++ b/include/circt/Dialect/Sim/SimOps.td @@ -96,21 +96,21 @@ def DPIFuncOp : SimOp<"func.dpi", }]; } -def DPICallOp : SimOp<"func.dpi.call", +def CallOp : SimOp<"func.call", [CallOpInterface, AttrSizedOperandSegments, DeclareOpInterfaceMethods]> { - 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. If provided, the DPI function is called at clock's posedge. The result values behave 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 + 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 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 diff --git a/integration_test/arcilator/JIT/dpi.mlir b/integration_test/arcilator/JIT/dpi.mlir index 8d3d5406ff1b..b57a7697fc3d 100644 --- a/integration_test/arcilator/JIT/dpi.mlir +++ b/integration_test/arcilator/JIT/dpi.mlir @@ -12,7 +12,7 @@ func.func @adder_func(%arg0: i32, %arg1: i32, %arg2: !llvm.ptr) { hw.module @adder(in %clock : i1, in %a : i32, in %b : i32, out c : i32) { %seq_clk = seq.to_clock %clock - %0 = sim.func.dpi.call @dpi(%a, %b) clock %seq_clk : (i32, i32) -> i32 + %0 = sim.func.call @dpi(%a, %b) clock %seq_clk : (i32, i32) -> i32 hw.output %0 : i32 } func.func @main() { diff --git a/lib/Conversion/ConvertToArcs/ConvertToArcs.cpp b/lib/Conversion/ConvertToArcs/ConvertToArcs.cpp index 14d01aa843a6..7aa4e74262c5 100644 --- a/lib/Conversion/ConvertToArcs/ConvertToArcs.cpp +++ b/lib/Conversion/ConvertToArcs/ConvertToArcs.cpp @@ -26,7 +26,7 @@ using llvm::MapVector; static bool isArcBreakingOp(Operation *op) { return op->hasTrait() || isa(op) || + seq::ClockGateOp, sim::CallOp>(op) || op->getNumResults() > 1; } diff --git a/lib/Conversion/SimToSV/SimToSV.cpp b/lib/Conversion/SimToSV/SimToSV.cpp index a1d0af51ab42..e0016549cfda 100644 --- a/lib/Conversion/SimToSV/SimToSV.cpp +++ b/lib/Conversion/SimToSV/SimToSV.cpp @@ -165,12 +165,12 @@ class SimulatorStopLowering : public SimConversionPattern { } }; -class DPICallLowering : public SimConversionPattern { +class DPICallLowering : public SimConversionPattern { public: - using SimConversionPattern::SimConversionPattern; + using SimConversionPattern::SimConversionPattern; LogicalResult - matchAndRewrite(DPICallOp op, OpAdaptor adaptor, + matchAndRewrite(sim::CallOp op, OpAdaptor adaptor, ConversionPatternRewriter &rewriter) const final { auto loc = op.getLoc(); // Record the callee. diff --git a/lib/Dialect/Arc/Transforms/LowerState.cpp b/lib/Dialect/Arc/Transforms/LowerState.cpp index 7f5025b8e457..b0296391ce2a 100644 --- a/lib/Dialect/Arc/Transforms/LowerState.cpp +++ b/lib/Dialect/Arc/Transforms/LowerState.cpp @@ -123,7 +123,7 @@ struct ModuleLowering { Value reset, ArrayRef inputs, FlatSymbolRefAttr callee); LogicalResult lowerState(StateOp stateOp); - LogicalResult lowerState(sim::DPICallOp dpiCallOp); + LogicalResult lowerState(sim::CallOp dpiCallOp); LogicalResult lowerState(MemoryOp memOp); LogicalResult lowerState(MemoryWritePortOp memWriteOp); LogicalResult lowerState(TapOp tapOp); @@ -145,7 +145,7 @@ static bool shouldMaterialize(Operation *op) { return !isa(op); + StateOp, sim::CallOp>(op); } static bool shouldMaterialize(Value value) { @@ -396,14 +396,14 @@ LogicalResult ModuleLowering::lowerPrimaryOutputs() { LogicalResult ModuleLowering::lowerStates() { SmallVector opsToLower; for (auto &op : *moduleOp.getBodyBlock()) - if (isa(&op)) + if (isa(&op)) opsToLower.push_back(&op); for (auto *op : opsToLower) { LLVM_DEBUG(llvm::dbgs() << "- Lowering " << *op << "\n"); auto result = TypeSwitch(op) - .Case( + .Case( [&](auto op) { return lowerState(op); }) .Default(success()); if (failed(result)) @@ -507,7 +507,7 @@ LogicalResult ModuleLowering::lowerState(StateOp stateOp) { stateInputs, stateOp.getArcAttr()); } -LogicalResult ModuleLowering::lowerState(sim::DPICallOp callOp) { +LogicalResult ModuleLowering::lowerState(sim::CallOp callOp) { // Clocked call op can be considered as arc state with single latency. auto stateClock = callOp.getClock(); if (!stateClock) diff --git a/lib/Dialect/FIRRTL/Transforms/LowerDPI.cpp b/lib/Dialect/FIRRTL/Transforms/LowerDPI.cpp index b8a5b3c760cc..69d906b4cf29 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerDPI.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerDPI.cpp @@ -185,7 +185,7 @@ LogicalResult LowerDPI::lower() { outputTypes.push_back( lowerDPIArgumentType(dpiOp.getResult().getType())); - auto call = builder.create( + auto call = builder.create( outputTypes, firstDPIDecl.getSymNameAttr(), clock, enable, inputs); if (!call.getResults().empty()) { // Insert unrealized conversion cast HW type to FIRRTL type. diff --git a/lib/Dialect/Sim/SimOps.cpp b/lib/Dialect/Sim/SimOps.cpp index d0e304fa85be..c427e213ab14 100644 --- a/lib/Dialect/Sim/SimOps.cpp +++ b/lib/Dialect/Sim/SimOps.cpp @@ -16,6 +16,7 @@ #include "mlir/Dialect/Func/IR/FuncOps.h" #include "mlir/IR/PatternMatch.h" #include "mlir/Interfaces/FunctionImplementation.h" +#include "mlir/Interfaces/FunctionInterfaces.h" using namespace mlir; using namespace circt; @@ -69,16 +70,41 @@ ParseResult DPIFuncOp::parse(OpAsmParser &parser, OperationState &result) { } LogicalResult -sim::DPICallOp::verifySymbolUses(SymbolTableCollection &symbolTable) { +sim::CallOp::verifySymbolUses(SymbolTableCollection &symbolTable) { auto referencedOp = symbolTable.lookupNearestSymbolFrom(*this, getCalleeAttr()); if (!referencedOp) return emitError("cannot find function declaration '") << getCallee() << "'"; - if (isa(referencedOp)) - return success(); - return emitError("callee must be 'sim.dpi.func' or 'func.func' but got '") - << referencedOp->getName() << "'"; + if (!isa(referencedOp)) + return emitError("callee must be 'sim.dpi.func' or 'func.func' but got '") + << referencedOp->getName() << "'"; + + // Verify that the operand and result types match the callee. + auto fnType = cast( + cast(referencedOp).getFunctionType()); + + if (fnType.getNumInputs() != getInputs().size()) + return emitOpError("incorrect number of operands for callee"); + + for (unsigned i = 0, e = fnType.getNumInputs(); i != e; ++i) + if (getInputs()[i].getType() != fnType.getInput(i)) + return emitOpError("operand type mismatch: expected operand type ") + << fnType.getInput(i) << ", but provided " + << getInputs()[i].getType() << " for operand number " << i; + + if (fnType.getNumResults() != getNumResults()) + return emitOpError("incorrect number of results for callee"); + + for (unsigned i = 0, e = fnType.getNumResults(); i != e; ++i) + if (getResult(i).getType() != fnType.getResult(i)) { + auto diag = emitOpError("result type mismatch at index ") << i; + diag.attachNote() << " op result types: " << getResultTypes(); + diag.attachNote(referencedOp->getLoc()) << "function result types: " << fnType.getResults(); + return diag; + } + + return success(); } void DPIFuncOp::print(OpAsmPrinter &p) { diff --git a/lib/Dialect/Sim/Transforms/LowerDPIFunc.cpp b/lib/Dialect/Sim/Transforms/LowerDPIFunc.cpp index dd65b2bcaa8a..56d4bf2c6649 100644 --- a/lib/Dialect/Sim/Transforms/LowerDPIFunc.cpp +++ b/lib/Dialect/Sim/Transforms/LowerDPIFunc.cpp @@ -164,7 +164,7 @@ LogicalResult LowerDPIFuncPass::lowerDPI() { if (failed(lowerDPIFuncOp(simFunc, state, symbolTable))) return failure(); - op.walk([&](sim::DPICallOp op) { + op.walk([&](sim::CallOp op) { auto func = state.dpiFuncDeclMapping.at(op.getCalleeAttr().getAttr()); op.setCallee(func.getSymNameAttr()); }); diff --git a/test/Conversion/SimToSV/dpi.mlir b/test/Conversion/SimToSV/dpi.mlir index c352d5907ade..babae404875b 100644 --- a/test/Conversion/SimToSV/dpi.mlir +++ b/test/Conversion/SimToSV/dpi.mlir @@ -19,7 +19,7 @@ sim.func.dpi @dpi(out arg0: i1, in %arg1: i1, out arg2: i1) hw.module @dpi_call(in %clock : !seq.clock, in %enable : i1, in %in: i1, out o1: i1, out o2: i1, out o3: i1, out o4: i1, out o5: i1, out o6: i1, out o7: i1, out o8: i1) { - %0, %1 = sim.func.dpi.call @dpi(%in) clock %clock enable %enable: (i1) -> (i1, i1) + %0, %1 = sim.func.call @dpi(%in) clock %clock enable %enable: (i1) -> (i1, i1) // CHECK: %[[CLK:.+]] = seq.from_clock %clock // CHECK-NEXT: sv.always posedge %[[CLK]] { // CHECK-NEXT: sv.if %enable { @@ -36,7 +36,7 @@ hw.module @dpi_call(in %clock : !seq.clock, in %enable : i1, in %in: i1, // VERILOG-NEXT: end // VERILOG-NEXT: end - %2, %3 = sim.func.dpi.call @dpi(%in) clock %clock : (i1) -> (i1, i1) + %2, %3 = sim.func.call @dpi(%in) clock %clock : (i1) -> (i1, i1) // CHECK: %[[CLK:.+]] = seq.from_clock %clock // CHECK-NEXT: sv.always posedge %[[CLK]] { // CHECK-NEXT: %[[RESULT:.+]]:2 = sv.func.call.procedural @dpi(%in) : (i1) -> (i1, i1) @@ -49,7 +49,7 @@ hw.module @dpi_call(in %clock : !seq.clock, in %enable : i1, in %in: i1, // VERILOG-NEXT: [[RESULT_3:_.+]] <= [[TMP_RESULT_1]]; // VERILOG-NEXT: end - %4, %5 = sim.func.dpi.call @dpi(%in) enable %enable : (i1) -> (i1, i1) + %4, %5 = sim.func.call @dpi(%in) enable %enable : (i1) -> (i1, i1) // CHECK: sv.alwayscomb { // CHECK-NEXT: sv.if %enable { // CHECK-NEXT: %[[RESULT:.+]]:2 = sv.func.call.procedural @dpi(%in) : (i1) -> (i1, i1) @@ -72,7 +72,7 @@ hw.module @dpi_call(in %clock : !seq.clock, in %enable : i1, in %in: i1, // VERILOG-NEXT: end // VERILOG-NEXT: end - %6, %7 = sim.func.dpi.call @dpi(%in) : (i1) -> (i1, i1) + %6, %7 = sim.func.call @dpi(%in) : (i1) -> (i1, i1) // CHECK: sv.alwayscomb { // CHECK-NEXT: %[[RESULT:.+]]:2 = sv.func.call.procedural @dpi(%in) : (i1) -> (i1, i1) // CHECK-NEXT: sv.bpassign %{{.+}}, %[[RESULT]]#0 : i1 @@ -101,7 +101,7 @@ hw.module @Issue7191(out result : i32) { // CHECK: call.procedural @create_counter // CHECK: call.procedural @increment_counter - %0 = sim.func.dpi.call @create_counter() : () -> i64 - %1 = sim.func.dpi.call @increment_counter(%0) : (i64) -> i32 + %0 = sim.func.call @create_counter() : () -> i64 + %1 = sim.func.call @increment_counter(%0) : (i64) -> i32 hw.output %1 : i32 } diff --git a/test/Dialect/Arc/lower-state.mlir b/test/Dialect/Arc/lower-state.mlir index b312bb115c8d..d7856251a9d4 100644 --- a/test/Dialect/Arc/lower-state.mlir +++ b/test/Dialect/Arc/lower-state.mlir @@ -359,7 +359,7 @@ func.func private @func(%arg0: i32, %arg1: i32) -> i32 // CHECK-LABEL: arc.model @adder hw.module @adder(in %clock : i1, in %a : i32, in %b : i32, out c : i32) { %0 = seq.to_clock %clock - %1 = sim.func.dpi.call @func(%a, %b) clock %0 : (i32, i32) -> i32 + %1 = sim.func.call @func(%a, %b) clock %0 : (i32, i32) -> i32 // CHECK: arc.clock_tree // CHECK-NEXT: %[[A:.+]] = arc.state_read %in_a : // CHECK-NEXT: %[[B:.+]] = arc.state_read %in_b : diff --git a/test/Dialect/FIRRTL/lower-dpi.mlir b/test/Dialect/FIRRTL/lower-dpi.mlir index 63105c884330..07a74baedf71 100644 --- a/test/Dialect/FIRRTL/lower-dpi.mlir +++ b/test/Dialect/FIRRTL/lower-dpi.mlir @@ -11,17 +11,17 @@ firrtl.circuit "DPI" { // CHECK-NEXT: %1 = builtin.unrealized_conversion_cast %enable : !firrtl.uint<1> to i1 // CHECK-NEXT: %2 = builtin.unrealized_conversion_cast %in_0 : !firrtl.uint<8> to i8 // CHECK-NEXT: %3 = builtin.unrealized_conversion_cast %in_1 : !firrtl.uint<8> to i8 - // CHECK-NEXT: %4 = sim.func.dpi.call @clocked_result(%2, %3) clock %0 enable %1 : (i8, i8) -> i8 + // CHECK-NEXT: %4 = sim.func.call @clocked_result(%2, %3) clock %0 enable %1 : (i8, i8) -> i8 // CHECK-NEXT: %5 = builtin.unrealized_conversion_cast %4 : i8 to !firrtl.uint<8> // CHECK-NEXT: %6 = builtin.unrealized_conversion_cast %clock : !firrtl.clock to !seq.clock // CHECK-NEXT: %7 = builtin.unrealized_conversion_cast %enable : !firrtl.uint<1> to i1 // CHECK-NEXT: %8 = builtin.unrealized_conversion_cast %in_0 : !firrtl.uint<8> to i8 // CHECK-NEXT: %9 = builtin.unrealized_conversion_cast %in_1 : !firrtl.uint<8> to i8 - // CHECK-NEXT: sim.func.dpi.call @clocked_void(%8, %9) clock %6 enable %7 : (i8, i8) -> () + // CHECK-NEXT: sim.func.call @clocked_void(%8, %9) clock %6 enable %7 : (i8, i8) -> () // CHECK-NEXT: %10 = builtin.unrealized_conversion_cast %enable : !firrtl.uint<1> to i1 // CHECK-NEXT: %11 = builtin.unrealized_conversion_cast %in_0 : !firrtl.uint<8> to i8 // CHECK-NEXT: %12 = builtin.unrealized_conversion_cast %in_1 : !firrtl.uint<8> to i8 - // CHECK-NEXT: %13 = sim.func.dpi.call @unclocked_result(%11, %12) enable %10 : (i8, i8) -> i8 + // CHECK-NEXT: %13 = sim.func.call @unclocked_result(%11, %12) enable %10 : (i8, i8) -> i8 // CHECK-NEXT: %14 = builtin.unrealized_conversion_cast %13 : i8 to !firrtl.uint<8> // CHECK-NEXT: firrtl.matchingconnect %out_0, %5 : !firrtl.uint<8> // CHECK-NEXT: firrtl.matchingconnect %out_1, %14 : !firrtl.uint<8> diff --git a/test/Dialect/Sim/lower-dpi.mlir b/test/Dialect/Sim/lower-dpi.mlir index dc9211f126dd..1bbd22661ef2 100644 --- a/test/Dialect/Sim/lower-dpi.mlir +++ b/test/Dialect/Sim/lower-dpi.mlir @@ -38,13 +38,13 @@ sim.func.dpi @baz(out arg0: i32, in %arg1: i32, out arg2: i32) attributes {veril // CHECK-LABEL: hw.module @dpi_call hw.module @dpi_call(in %clock : !seq.clock, in %enable : i1, in %in: i32, out o1: i32, out o2: i32, out o3: i32, out o4: i32, out o5: i32, out o6: i32) { - // CHECK-NEXT: %0:2 = sim.func.dpi.call @foo_wrapper(%in) clock %clock : (i32) -> (i32, i32) - // CHECK-NEXT: %1:2 = sim.func.dpi.call @bar_wrapper(%in) : (i32) -> (i32, i32) - // CHECK-NEXT: %2:2 = sim.func.dpi.call @baz_wrapper(%in) : (i32) -> (i32, i32) + // CHECK-NEXT: %0:2 = sim.func.call @foo_wrapper(%in) clock %clock : (i32) -> (i32, i32) + // CHECK-NEXT: %1:2 = sim.func.call @bar_wrapper(%in) : (i32) -> (i32, i32) + // CHECK-NEXT: %2:2 = sim.func.call @baz_wrapper(%in) : (i32) -> (i32, i32) // CHECK-NEXT: hw.output %0#0, %0#1, %1#0, %1#1, %2#0, %2#1 : i32, i32, i32, i32, i32, i32 - %0, %1 = sim.func.dpi.call @foo(%in) clock %clock : (i32) -> (i32, i32) - %2, %3 = sim.func.dpi.call @bar(%in) : (i32) -> (i32, i32) - %4, %5 = sim.func.dpi.call @baz(%in) : (i32) -> (i32, i32) + %0, %1 = sim.func.call @foo(%in) clock %clock : (i32) -> (i32, i32) + %2, %3 = sim.func.call @bar(%in) : (i32) -> (i32, i32) + %4, %5 = sim.func.call @baz(%in) : (i32) -> (i32, i32) hw.output %0, %1, %2, %3, %4, %5 : i32, i32, i32, i32, i32, i32 } diff --git a/test/Dialect/Sim/round-trip.mlir b/test/Dialect/Sim/round-trip.mlir index bbbe07d4d931..f9b766f19c9d 100644 --- a/test/Dialect/Sim/round-trip.mlir +++ b/test/Dialect/Sim/round-trip.mlir @@ -22,12 +22,12 @@ sim.func.dpi @dpi(out arg0: i1, in %arg1: i1, out arg2: i1) func.func private @func(%arg1: i1) -> (i1, i1) hw.module @dpi_call(in %clock : !seq.clock, in %enable : i1, in %in: i1) { - // CHECK: sim.func.dpi.call @dpi(%in) clock %clock enable %enable : (i1) -> (i1, i1) - %0, %1 = sim.func.dpi.call @dpi(%in) clock %clock enable %enable: (i1) -> (i1, i1) - // CHECK: sim.func.dpi.call @dpi(%in) clock %clock : (i1) -> (i1, i1) - %2, %3 = sim.func.dpi.call @dpi(%in) clock %clock : (i1) -> (i1, i1) - // CHECK: sim.func.dpi.call @func(%in) enable %enable : (i1) -> (i1, i1) - %4, %5 = sim.func.dpi.call @func(%in) enable %enable : (i1) -> (i1, i1) - // CHECK: sim.func.dpi.call @func(%in) : (i1) -> (i1, i1) - %6, %7 = sim.func.dpi.call @func(%in) : (i1) -> (i1, i1) + // CHECK: sim.func.call @dpi(%in) clock %clock enable %enable : (i1) -> (i1, i1) + %0, %1 = sim.func.call @dpi(%in) clock %clock enable %enable: (i1) -> (i1, i1) + // CHECK: sim.func.call @dpi(%in) clock %clock : (i1) -> (i1, i1) + %2, %3 = sim.func.call @dpi(%in) clock %clock : (i1) -> (i1, i1) + // CHECK: sim.func.call @func(%in) enable %enable : (i1) -> (i1, i1) + %4, %5 = sim.func.call @func(%in) enable %enable : (i1) -> (i1, i1) + // CHECK: sim.func.call @func(%in) : (i1) -> (i1, i1) + %6, %7 = sim.func.call @func(%in) : (i1) -> (i1, i1) } diff --git a/test/Dialect/Sim/sim-errors.mlir b/test/Dialect/Sim/sim-errors.mlir index a70360b1cdfe..68770c8d867a 100644 --- a/test/Dialect/Sim/sim-errors.mlir +++ b/test/Dialect/Sim/sim-errors.mlir @@ -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) }