[Sim] Replace sim.func.dpi.call with sim.func.call and improve a veri…

…fier

Just drop `.dpi` Since sim.func.dpi.call now accepts func.func as well.

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<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.
     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

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() {

lib/Conversion/ConvertToArcs/ConvertToArcs.cpp

@@ -26,7 +26,7 @@ using llvm::MapVector;
 static bool isArcBreakingOp(Operation *op) {
   return op->hasTrait<OpTrait::ConstantLike>() ||
          isa<hw::InstanceOp, seq::CompRegOp, MemoryOp, ClockedOpInterface,
-             seq::ClockGateOp, sim::DPICallOp>(op) ||
+             seq::ClockGateOp, sim::CallOp>(op) ||
          op->getNumResults() > 1;
 }
 

lib/Conversion/SimToSV/SimToSV.cpp

@@ -165,12 +165,12 @@ class SimulatorStopLowering : public SimConversionPattern<FromOp> {
   }
 };
 
-class DPICallLowering : public SimConversionPattern<DPICallOp> {
+class DPICallLowering : public SimConversionPattern<sim::CallOp> {
 public:
-  using SimConversionPattern<DPICallOp>::SimConversionPattern;
+  using SimConversionPattern<sim::CallOp>::SimConversionPattern;
 
   LogicalResult
-  matchAndRewrite(DPICallOp op, OpAdaptor adaptor,
+  matchAndRewrite(sim::CallOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const final {
     auto loc = op.getLoc();
     // Record the callee.

lib/Dialect/Arc/Transforms/LowerState.cpp

@@ -123,7 +123,7 @@ struct ModuleLowering {
                                Value reset, ArrayRef<Value> 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<MemoryOp, AllocStateOp, AllocMemoryOp, AllocStorageOp,
               ClockTreeOp, PassThroughOp, RootInputOp, RootOutputOp,
               StateWriteOp, MemoryWritePortOp, igraph::InstanceOpInterface,
-              StateOp, sim::DPICallOp>(op);
+              StateOp, sim::CallOp>(op);
 }
 
 static bool shouldMaterialize(Value value) {
@@ -396,14 +396,14 @@ LogicalResult ModuleLowering::lowerPrimaryOutputs() {
 LogicalResult ModuleLowering::lowerStates() {
   SmallVector<Operation *> opsToLower;
   for (auto &op : *moduleOp.getBodyBlock())
-    if (isa<StateOp, MemoryOp, MemoryWritePortOp, TapOp, sim::DPICallOp>(&op))
+    if (isa<StateOp, MemoryOp, MemoryWritePortOp, TapOp, sim::CallOp>(&op))
       opsToLower.push_back(&op);
 
   for (auto *op : opsToLower) {
     LLVM_DEBUG(llvm::dbgs() << "- Lowering " << *op << "\n");
     auto result =
         TypeSwitch<Operation *, LogicalResult>(op)
-            .Case<StateOp, MemoryOp, MemoryWritePortOp, TapOp, sim::DPICallOp>(
+            .Case<StateOp, MemoryOp, MemoryWritePortOp, TapOp, sim::CallOp>(
                 [&](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)

lib/Dialect/FIRRTL/Transforms/LowerDPI.cpp

@@ -185,7 +185,7 @@ LogicalResult LowerDPI::lower() {
         outputTypes.push_back(
             lowerDPIArgumentType(dpiOp.getResult().getType()));
 
-      auto call = builder.create<sim::DPICallOp>(
+      auto call = builder.create<sim::CallOp>(
           outputTypes, firstDPIDecl.getSymNameAttr(), clock, enable, inputs);
       if (!call.getResults().empty()) {
         // Insert unrealized conversion cast HW type to FIRRTL type.

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,42 @@ 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<func::FuncOp, sim::DPIFuncOp>(referencedOp))
-    return success();
-  return emitError("callee must be 'sim.dpi.func' or 'func.func' but got '")
-         << referencedOp->getName() << "'";
+  if (!isa<func::FuncOp, sim::DPIFuncOp>(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<FunctionType>(
+      cast<mlir::FunctionOpInterface>(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) {

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());
   });

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
 }

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 : <i32>
   // CHECK-NEXT:   %[[B:.+]] = arc.state_read %in_b : <i32>

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>

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
 }

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)
 }

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)
 }