Conversion of FIRRTL CircuitOp

Hello everyone,
I am to implementing my own variant of the FIRRTL dialect to focus on security research and try to convert a circt.firrtl.CircuitOp to my own CircuitOp. For that I have written an OpConversionPattern:

struct FIRRTLCircuitOpConversion : public mlir::OpConversionPattern<firrtl::CircuitOp> {
    using mlir::OpConversionPattern<firrtl::CircuitOp>::OpConversionPattern;

    mlir::LogicalResult matchAndRewrite(firrtl::CircuitOp firrtlCircuitOp, mlir::ArrayRef<mlir::Value> operands, mlir::ConversionPatternRewriter &rewriter) const final {
        llvm::StringRef name("TestCircuit");
        //Create a SecFIR circuit operation with the same name as the FIRRTL circuit operation    
        secfir::CircuitOp secfirCircuitOp = rewriter.create<secfir::CircuitOp>(firrtlCircuitOp.getLoc(), rewriter.getStringAttr(name));
        //Set instertion point to the beginning of SecFIR circuit
        rewriter.setInsertionPointToStart(secfirCircuitOp.getBody());                        
        //Move content from old circuit to new circuit
        rewriter.inlineRegionBefore(firrtlCircuitOp.getRegion(), secfirCircuitOp.getRegion(),  secfirCircuitOp.getRegion().begin());
        //Delete old circuit
        rewriter.eraseOp(firrtlCircuitOp);    
        return mlir::success();
   }

Applying this to a simple test program seems to correctly translate the circuit operation:

//===-------------------------------------------===//
Legalizing operation : 'firrtl.circuit'(0x5555562f7f30) {
  * Fold {
  } -> FAILURE : unable to fold

  * Pattern : 'firrtl.circuit -> ()' {
    ** Insert  : 'secfir.done'(0x55555630fd50)
    ** Insert  : 'secfir.circuit'(0x55555630fdc0)
    ** Erase   : 'firrtl.circuit'(0x5555562f7f30)
    //===-------------------------------------------===//
    Legalizing operation : 'secfir.done'(0x55555630fd50) {
      "secfir.done"() : () -> ()
    } -> SUCCESS : operation marked legal by the target
    //===-------------------------------------------===//

    //===-------------------------------------------===//
    Legalizing operation : 'secfir.circuit'(0x55555630fdc0) {
    } -> SUCCESS : operation marked legal by the target
    //===-------------------------------------------===//
  } -> SUCCESS : pattern applied successfully
} -> SUCCESS
//===-------------------------------------------===//

But at the same time I get some error, suggesting that the firrtl.CircuitOp was not erased properly:

test/Test.lo.fir:1:1: error: 'firrtl.circuit' op Operations with a 'SymbolTable' must have exactly one block
circuit sbox :
^
test/Test.lo.fir:1:1: note: see current operation: "firrtl.circuit"() ( {
}) {name = "sbox"} : () -> ()
module {
  secfir.circuit "TestCircuit" {
    firrtl.module @sbox(%clock: !firrtl.clock, %a: !firrtl.uint<1>, %b: !firrtl.flip<uint<1>>) {
      %0 = firrtl.not %a : (!firrtl.uint<1>) -> !firrtl.uint<1>
      firrtl.connect %b, %0 : !firrtl.flip<uint<1>>, !firrtl.uint<1>
    }
  ^bb1:  // no predecessors
    "secfir.done"() : () -> ()
  }
}test/Test.lo.fir:1:1: error: unknown operation
circuit sbox :
^
test/Test.lo.fir:1:1: note: see current operation: "secfir.circuit"() ( {
  "firrtl.module"() ( {
  ^bb0(%clock: !firrtl.clock, %a: !firrtl.uint<1>, %b: !firrtl.flip<uint<1>>):  // no predecessors
    %0 = "firrtl.not"(%a) : (!firrtl.uint<1>) -> !firrtl.uint<1>
    "firrtl.connect"(%b, %0) : (!firrtl.flip<uint<1>>, !firrtl.uint<1>) -> ()
    "firrtl.done"() : () -> ()
  }) {arg0 = {firrtl.name = "clock"}, arg1 = {firrtl.name = "a"}, arg2 = {firrtl.name = "b"}, sym_name = "sbox", type = (!firrtl.clock, !firrtl.uint<1>, !firrtl.flip<uint<1>>) -> ()} : () -> ()
  "firrtl.done"() : () -> ()
^bb1:  // no predecessors
  "secfir.done"() : () -> ()
}) {name = "TestCircuit"} : () -> ()

Any idea what goes wrong? The circuit operation is the root operation of the entire program, does this require a special handling? I would really appreciate any help. Thanks!

I would say that this is because secfir::CircuitOp automatically creates a block inside of its region for you when you call create<secfir::CircuitOp>. When you later do inlineRegionBefore it just adds the blocks from firrtlCircuitOp.getRegion() before the one that already exists in secfirCircuitOp. I suspect that you either want to a) Delete the original block that gets created automatically, or b) Use something like mergeBlocks instead of inlineRegion. (disclaimer: I don’t know anything about CIRCT ops, just basing this off of the code sample you have)

– River

Thanks, this is a good thought and indeed what I would like to do. But it does not seem to be the problem at hand and the errors still appear. I assume the secfir::CircuitOp is created within the firrtl::CircuitOp, as indicated in the first error message, and I don’t know how to set the secfir::CircuitOpas the new root.

The reason seems to be that I take the region out of the original firrtl.CircuitOp, such that this operation is empty. This seems to be an issue even if the operation gets removed afterward and it is not enough to clone the region back. I’m not entirely sure when this gets checked but I can simply ignore the error and everything works just fine. But to me, this looks like an unsatisfying solution.