[Ibis] Split ContainerOp in two

include/circt/Dialect/Ibis/IbisInterfaces.td

@@ -168,4 +168,42 @@ def MethodLikeOpInterface : OpInterface<"MethodLikeOpInterface"> {
   }];
 }
 
+def ContainerOpInterface : OpInterface<"ContainerOpInterface"> {
+  let cppNamespace = "circt::ibis";
+  let description = [{
+    An interface for Ibis operations that contain other operations.
+  }];
+
+  let methods = [
+    InterfaceMethod<
+      "Returns the body of the container",
+      "mlir::Block*", "getBodyBlock",
+      (ins), [{}], /*defaultImplementation=*/ [{
+        return &$_op.getBody().front();
+      }]
+    >,
+    InterfaceMethod<
+      "Returns the symbol name",
+      "StringAttr", "getSymNameAttr",
+      (ins), [{
+        return $_op.getSymNameAttr();
+      }]
+    >,
+    InterfaceMethod<
+      "Returns the module name as a string attribute",
+      "StringAttr", "getModuleNameAttr",
+      (ins), [{
+        return $_op.getModuleNameAttr();
+      }]
+    >,
+    InterfaceMethod<
+      "Returns the module name",
+      "StringRef", "getModuleName",
+      (ins), [{
+        return $_op.getModuleNameAttr().getValue();
+      }]
+    >
+  ];
+}
+
 #endif // CIRCT_DIALECT_IBIS_INTERFACES_TD

include/circt/Dialect/Ibis/IbisOps.h

@@ -29,7 +29,8 @@
 #include "mlir/Interfaces/SideEffectInterfaces.h"
 namespace circt {
 namespace ibis {
-class ContainerOp;
+class OuterContainerOp;
+class InnerContainerOp;
 class ThisOp;
 
 // Symbol name for the ibis operator library to be used during scheduling.

include/circt/Dialect/Ibis/IbisOps.td

@@ -568,14 +568,12 @@ def ThisOp : IbisOp<"this", [
 // Low-level Ibis operations
 // ===---------------------------------------------------------------------===//
 
-def ContainerOp : IbisOp<"container", [
-  Symbol,
-  // TODO: @mortbopet: ContainerOp should be an innersymbol table (or equivalent)
-  // whenever we figure out how to support nested symbol tables...
-  // InnerSymbolTable
+class ContainerOp<string mnemonic, list<Trait> traits> : IbisOp<mnemonic,
+  traits # [
+  ContainerOpInterface,
   SingleBlock,
   NoTerminator, NoRegionArguments,
-  ScopeOpInterface, IsolatedFromAbove,
+  IsolatedFromAbove,
   InstanceGraphModuleOpInterface,
   RegionKindInterface
 ]> {
@@ -584,36 +582,57 @@ def ContainerOp : IbisOp<"container", [
     An ibis container describes a collection of logic nested within an Ibis class.
   }];
 
-  let arguments = (ins SymbolNameAttr:$sym_name);
   let regions = (region SizedRegion<1>:$body);
 
+  code extraContainerClassDeclaration = [{}];
+  let extraClassDeclaration = extraContainerClassDeclaration # [{
+    // Implement RegionKindInterface.
+    static RegionKind getRegionKind(unsigned index) { return RegionKind::Graph; }
+  }];
+
+  let skipDefaultBuilders = 1;
+}
+
+def OuterContainerOp : ContainerOp<"container.outer", [
+  Symbol, InnerSymbolTable, ScopeOpInterface]> {
+  let arguments = (ins SymbolNameAttr:$sym_name);
   let assemblyFormat = [{
     $sym_name attr-dict-with-keyword $body
   }];
 
-  let extraClassDeclaration = [{
-    // Implement RegionKindInterface.
-    static RegionKind getRegionKind(unsigned index) { return RegionKind::Graph; }
-
-    Block* getBodyBlock() { return &getBody().front(); }
+  let builders = [
+    OpBuilder<(ins "StringAttr":$symbol), [{
+      auto region = $_state.addRegion();
+      region->push_back(new Block());
+      $_state.addAttribute(getSymNameAttrName($_state.name), symbol);
+    }]>
+  ];
+}
 
-    StringAttr getModuleNameAttr() {
-      return getSymNameAttr();
-    }
 
-    llvm::StringRef getModuleName() {
-      return getSymName();
-    }
+def InnerContainerOp : ContainerOp<"container.inner", [
+  DeclareOpInterfaceMethods<InnerSymbol>]> {
+  let arguments = (ins InnerSymAttr:$inner_sym);
+  let assemblyFormat = [{
+    $inner_sym attr-dict-with-keyword $body
   }];
 
-  let skipDefaultBuilders = 1;
   let builders = [
-    OpBuilder<(ins "StringAttr":$name), [{
+    OpBuilder<(ins "::circt::hw::InnerSymAttr":$symbol), [{
       auto region = $_state.addRegion();
       region->push_back(new Block());
-      $_state.addAttribute(getSymNameAttrName($_state.name), name);
+      $_state.addAttribute(getInnerSymAttrName($_state.name), symbol);
     }]>
   ];
+
+  let extraContainerClassDeclaration = [{
+    StringAttr getSymNameAttr() {
+      return getInnerSymAttr().getSymName();
+    }
+    StringAttr getModuleNameAttr() {
+      return getSymNameAttr();
+    }
+  }];
 }
 
 def ContainerInstanceOp : InstanceOpBase<"container.instance"> {
@@ -624,7 +643,7 @@ def ContainerInstanceOp : InstanceOpBase<"container.instance"> {
 
   let extraInstanceClassDeclaration = [{
     // Return the container this instance is instantiating.
-    ContainerOp getContainer(const SymbolTable* symbolTable = nullptr);
+    ContainerOpInterface getContainer(const SymbolTable* symbolTable = nullptr);
 
     Operation* getReferencedModuleSlow();
     Operation* getReferencedModule(const SymbolTable&);
@@ -694,7 +713,7 @@ def GetPortOp : IbisOp<"get_port", [
 class PortLikeOp<string mnemonic, list<Trait> traits = []> :
     IbisOp<mnemonic, !listconcat(traits, [
       PortOpInterface,
-      ParentOneOf<["ClassOp", "ContainerOp"]>,
+      ParentOneOf<["ClassOp", "OuterContainerOp", "InnerContainerOp"]>,
       InferTypeOpInterface,
       HasCustomSSAName,
       DeclareOpInterfaceMethods<InnerSymbol,["getTargetResultIndex"]>
@@ -775,7 +794,7 @@ def OutputPortOp : PortLikeOp<"port.output"> {
 class WireLikeOp<string mnemonic, list<Trait> traits = []> :
     IbisOp<mnemonic, !listconcat(traits, [
       PortOpInterface,
-      ParentOneOf<["ClassOp", "ContainerOp"]>,
+      ParentOneOf<["ClassOp", "OuterContainerOp", "InnerContainerOp"]>,
       HasCustomSSAName,
       DeclareOpInterfaceMethods<InnerSymbol,["getTargetResultIndex"]>
   ])> {

lib/Dialect/Ibis/IbisOps.cpp

@@ -405,13 +405,14 @@ void PortReadOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
 // ContainerInstanceOp
 //===----------------------------------------------------------------------===//
 
-ContainerOp ContainerInstanceOp::getContainer(const SymbolTable *symbolTable) {
+ContainerOpInterface
+ContainerInstanceOp::getContainer(const SymbolTable *symbolTable) {
   auto mod = getOperation()->getParentOfType<mlir::ModuleOp>();
   if (symbolTable)
-    return dyn_cast_or_null<ContainerOp>(
+    return dyn_cast_or_null<ContainerOpInterface>(
         symbolTable->lookupSymbolIn(mod, getTargetNameAttr()));
 
-  return mod.lookupSymbol<ContainerOp>(getTargetNameAttr());
+  return mod.lookupSymbol<ContainerOpInterface>(getTargetNameAttr());
 }
 
 LogicalResult
@@ -428,6 +429,10 @@ void ContainerInstanceOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
   setNameFn(getResult(), genValueNameAttr(getResult()));
 }
 
+std::optional<size_t> InnerContainerOp::getTargetResultIndex() {
+  return std::nullopt;
+}
+
 //===----------------------------------------------------------------------===//
 // PathOp
 //===----------------------------------------------------------------------===//

lib/Dialect/Ibis/Transforms/IbisCleanSelfdrivers.cpp

@@ -38,7 +38,7 @@ namespace {
 
 // Rewrites cases where an input port is being read in the instantiating module.
 // Replaces the input port read by the assignment value of the input port.
-static LogicalResult replaceReadsOfWrites(ContainerOp containerOp) {
+static LogicalResult replaceReadsOfWrites(ContainerOpInterface containerOp) {
   // Partition out all of the get_port's wrt. their target port symbol.
   struct PortAccesses {
     GetPortOp getAsInput;
@@ -50,7 +50,7 @@ static LogicalResult replaceReadsOfWrites(ContainerOp containerOp) {
                  /*portName*/ llvm::DenseMap<StringAttr, PortAccesses>>
       instancePortAccessMap;
 
-  for (auto getPortOp : containerOp.getOps<GetPortOp>()) {
+  for (auto getPortOp : containerOp.getBodyBlock()->getOps<GetPortOp>()) {
     PortAccesses &portAccesses =
         instancePortAccessMap[getPortOp.getInstance()]
                              [getPortOp.getPortSymbolAttr().getAttr()];
@@ -190,7 +190,8 @@ struct CleanSelfdriversPass
 } // anonymous namespace
 
 LogicalResult CleanSelfdriversPass::cleanInstanceSide() {
-  for (ContainerOp containerOp : getOperation().getOps<ContainerOp>())
+  for (ContainerOpInterface containerOp :
+       getOperation().getOps<ContainerOpInterface>())
     if (failed(replaceReadsOfWrites(containerOp)))
       return failure();
 

lib/Dialect/Ibis/Transforms/IbisContainerize.cpp

@@ -22,14 +22,14 @@ using namespace ibis;
 
 namespace {
 
-struct OutlineContainerPattern : public OpConversionPattern<ContainerOp> {
+struct OutlineContainerPattern : public OpConversionPattern<InnerContainerOp> {
   OutlineContainerPattern(MLIRContext *context, Namespace &ns)
-      : OpConversionPattern<ContainerOp>(context), ns(ns) {}
+      : OpConversionPattern<InnerContainerOp>(context), ns(ns) {}
 
-  using OpAdaptor = typename OpConversionPattern<ContainerOp>::OpAdaptor;
+  using OpAdaptor = typename OpConversionPattern<InnerContainerOp>::OpAdaptor;
 
   LogicalResult
-  matchAndRewrite(ContainerOp op, OpAdaptor adaptor,
+  matchAndRewrite(InnerContainerOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
     // Outline the container into the module scope, by prefixing it with the
     // parent class name.
@@ -40,9 +40,9 @@ struct OutlineContainerPattern : public OpConversionPattern<ContainerOp> {
 
     rewriter.setInsertionPoint(parentClass);
     StringAttr newContainerName = rewriter.getStringAttr(
-        ns.newName(parentClass.getName() + "_" + op.getName()));
+        ns.newName(parentClass.getName() + "_" + op.getModuleName()));
     auto newContainer =
-        rewriter.create<ContainerOp>(op.getLoc(), newContainerName);
+        rewriter.create<OuterContainerOp>(op.getLoc(), newContainerName);
 
     rewriter.mergeBlocks(op.getBodyBlock(), newContainer.getBodyBlock(), {});
 
@@ -74,7 +74,7 @@ struct ClassToContainerPattern : public OpConversionPattern<ClassOp> {
                   ConversionPatternRewriter &rewriter) const override {
     // Replace the class by a container of the same name.
     auto newContainer =
-        rewriter.create<ContainerOp>(op.getLoc(), op.getNameAttr());
+        rewriter.create<OuterContainerOp>(op.getLoc(), op.getNameAttr());
     rewriter.mergeBlocks(op.getBodyBlock(), newContainer.getBodyBlock(), {});
     rewriter.eraseOp(op);
     return success();
@@ -112,7 +112,7 @@ LogicalResult ContainerizePass::outlineContainers() {
   auto *context = &getContext();
   ConversionTarget target(*context);
   target.addLegalDialect<IbisDialect>();
-  target.addDynamicallyLegalOp<ContainerOp>(
+  target.addDynamicallyLegalOp<InnerContainerOp>(
       [&](auto *op) { return !isa<ibis::ClassOp>(op->getParentOp()); });
   RewritePatternSet patterns(context);
 

lib/Dialect/Ibis/Transforms/IbisContainersToHW.cpp

@@ -39,7 +39,7 @@ struct ContainerPortInfo {
   llvm::DenseMap<StringAttr, OutputPortOp> opOutputs;
 
   ContainerPortInfo() = default;
-  ContainerPortInfo(ContainerOp container) {
+  ContainerPortInfo(ContainerOpInterface container) {
     SmallVector<hw::PortInfo, 4> inputs, outputs;
 
     // Copies all attributes from a port, except for the port symbol and type.
@@ -85,13 +85,14 @@ struct ContainerPortInfo {
 
 using ContainerPortInfoMap = llvm::DenseMap<StringAttr, ContainerPortInfo>;
 
+template <typename ContainerOp>
 struct ContainerOpConversionPattern : public OpConversionPattern<ContainerOp> {
   ContainerOpConversionPattern(MLIRContext *ctx,
                                ContainerPortInfoMap &portOrder)
       : OpConversionPattern<ContainerOp>(ctx), portOrder(portOrder) {}
 
   LogicalResult
-  matchAndRewrite(ContainerOp op, OpAdaptor adaptor,
+  matchAndRewrite(ContainerOp op, typename ContainerOp::Adaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
     rewriter.setInsertionPoint(op);
 
@@ -298,12 +299,13 @@ void ContainersToHWPass::runOnOperation() {
 
   // Generate module signatures.
   ContainerPortInfoMap portOrder;
-  for (auto container : getOperation().getOps<ContainerOp>())
+  for (auto container : getOperation().getOps<ContainerOpInterface>())
     portOrder.try_emplace(container.getSymNameAttr(),
                           ContainerPortInfo(container));
 
   ConversionTarget target(*ctx);
-  target.addIllegalOp<ContainerOp, ContainerInstanceOp, ThisOp>();
+  target.addIllegalOp<OuterContainerOp, InnerContainerOp, ContainerInstanceOp,
+                      ThisOp>();
   target.markUnknownOpDynamicallyLegal([](Operation *) { return true; });
 
   // Parts of the conversion patterns will update operations in place, which in
@@ -313,9 +315,9 @@ void ContainersToHWPass::runOnOperation() {
   target.addLegalDialect<IbisDialect>();
 
   RewritePatternSet patterns(ctx);
-  patterns
-      .add<ContainerOpConversionPattern, ContainerInstanceOpConversionPattern>(
-          ctx, portOrder);
+  patterns.add<ContainerOpConversionPattern<OuterContainerOp>,
+               ContainerOpConversionPattern<InnerContainerOp>,
+               ContainerInstanceOpConversionPattern>(ctx, portOrder);
   patterns.add<ThisOpConversionPattern>(ctx);
 
   if (failed(

lib/Dialect/Ibis/Transforms/IbisMethodsToContainers.cpp

@@ -31,8 +31,8 @@ struct DataflowMethodOpConversion
   matchAndRewrite(DataflowMethodOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
     // Replace the class by a container of the same name.
-    auto newContainer = rewriter.create<ContainerOp>(
-        op.getLoc(), op.getInnerSym().getSymName());
+    auto newContainer = rewriter.create<InnerContainerOp>(
+        op.getLoc(), hw::InnerSymAttr::get(op.getInnerSym().getSymName()));
     rewriter.setInsertionPointToStart(newContainer.getBodyBlock());
 
     // Create mandatory %this