[MSFT] Support for multiple top levels of same module (#3321)

Some designs instantiate the same module more than once. Per-instance
data should be different, so we need a way to specify an "instance" of
the top level

include/circt/Dialect/MSFT/ExportTcl.h

@@ -16,6 +16,8 @@
 #include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/MSFT/MSFTOpInterfaces.h"
 #include "circt/Support/LLVM.h"
+
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace circt {
@@ -30,13 +32,21 @@ public:
   LogicalResult emit(Operation *forMod, StringRef outputFile);
 
   Operation *getDefinition(FlatSymbolRefAttr);
+  const DenseSet<hw::GlobalRefOp> &getRefsUsed() { return refsUsed; }
+  void usedRef(hw::GlobalRefOp ref) { refsUsed.insert(ref); }
 
 private:
   mlir::ModuleOp topLevel;
 
   bool populated;
   hw::HWSymbolCache topLevelSymbols;
-  DenseMap<Operation *, SmallVector<DynInstDataOpInterface, 0>> tclOpsForMod;
+
+  /// Map Module operations to their top-level "instance" names. Map those
+  /// "instance" names to the lowered ops which get directly emitted as tcl.
+  DenseMap<Operation *,
+           llvm::MapVector<StringAttr, SmallVector<DynInstDataOpInterface, 0>>>
+      tclOpsForModInstance;
+  DenseSet<hw::GlobalRefOp> refsUsed;
 
   LogicalResult populate();
 };

include/circt/Dialect/MSFT/MSFTOpInterfaces.h

@@ -10,10 +10,12 @@
 #define CIRCT_DIALECT_MSFT_MSFTOPINTERFACES_H
 
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 
 namespace circt {
 namespace msft {
 LogicalResult verifyDynInstData(Operation *);
+class InstanceHierarchyOp;
 } // namespace msft
 } // namespace circt
 #include "circt/Dialect/MSFT/MSFTOpInterfaces.h.inc"

include/circt/Dialect/MSFT/MSFTOpInterfaces.td

@@ -41,6 +41,33 @@ def DynInstDataOpInterface : OpInterface<"DynInstDataOpInterface"> {
       /*defaultImplementation=*/[{
         return $_op.refAttr();
       }]
+    >,
+    InterfaceMethod<
+      /*desc=*/[{
+        Get the top module op to which the GlobalRefOp which this op is referring.
+      }],
+      /*retTy=*/"Operation *",
+      /*methodName=*/"getTopModule",
+      /*args=*/(ins "circt::hw::HWSymbolCache &":$symCache),
+      /*methodBody=*/[{}],
+      /*defaultImplementation=*/[{
+        FlatSymbolRefAttr refSym = $_op.getGlobalRefSym();
+        if (!refSym) {
+          $_op->emitOpError("must run dynamic instance lowering first");
+          return nullptr;
+        }
+        auto ref = dyn_cast_or_null<hw::GlobalRefOp>(
+            symCache.getDefinition(refSym));
+        if (!ref) {
+          $_op->emitOpError("could not find hw.globalRef ") << refSym;
+          return nullptr;
+        }
+        if (ref.namepath().empty())
+          return nullptr;
+        auto modSym = FlatSymbolRefAttr::get(
+            ref.namepath()[0].cast<hw::InnerRefAttr>().getModule());
+        return symCache.getDefinition(modSym);
+      }]
     >
   ];
 }

include/circt/Dialect/MSFT/MSFTPDOps.td

@@ -82,12 +82,20 @@ def PDPhysRegionOp : MSFTOp<"pd.physregion",
 def InstanceHierarchyOp : MSFTOp<"instance.hierarchy",
                                  [HasParent<"mlir::ModuleOp">, NoTerminator]> {
   let summary = "The root of an instance hierarchy";
+  let description = [{
+    Models the "root" / "top" of an instance hierarchy. `DynamicInstanceOp`s
+    must be contained by this op. Specifies the top module and (optionally) an
+    "instance" name in the case where there are multiple instances of a
+    particular module in a design. (As is often the case where one isn't
+    producing the design's "top" module but a subdesign.)
+  }];
 
-  let arguments = (ins FlatSymbolRefAttr:$topModuleRef);
+  let arguments = (ins FlatSymbolRefAttr:$topModuleRef,
+                       OptionalAttr<StrAttr>:$instName);
   let regions = (region SizedRegion<1>:$body);
 
   let assemblyFormat = [{
-    $topModuleRef $body attr-dict
+    $topModuleRef ($instName^)? $body attr-dict
   }];
 }
 

lib/Dialect/MSFT/ExportQuartusTcl.cpp

@@ -46,24 +46,27 @@ LogicalResult TclEmitter::populate() {
   populated = true;
 
   // Bin any operations we may need to emit based on the root module in the
-  // instance hierarchy path.
-  for (auto tclOp : topLevel.getOps<DynInstDataOpInterface>()) {
-    FlatSymbolRefAttr refSym = tclOp.getGlobalRefSym();
-    if (!refSym)
-      return tclOp->emitOpError("must run dynamic instance lowering first");
-    auto ref = dyn_cast_or_null<hw::GlobalRefOp>(
-        topLevelSymbols.getDefinition(refSym));
-    if (!ref)
-      return tclOp->emitOpError("could not find hw.globalRef ") << refSym;
-    if (ref.namepath().empty())
-      continue;
-    auto modSym = FlatSymbolRefAttr::get(
-        ref.namepath()[0].cast<hw::InnerRefAttr>().getModule());
-    Operation *mod = topLevelSymbols.getDefinition(modSym);
-    assert(mod &&
-           "Invalid IR -- should have been caught by GlobalRef verifier");
-    tclOpsForMod[mod].push_back(tclOp);
+  // instance hierarchy path and the potential instance name.
+
+  // Look in InstanceHierarchyOps to get the instance named ones.
+  for (auto hier : topLevel.getOps<InstanceHierarchyOp>()) {
+    Operation *mod = topLevelSymbols.getDefinition(hier.topModuleRefAttr());
+    auto &tclOps = tclOpsForModInstance[mod][hier.instNameAttr()];
+    for (auto tclOp : hier.getOps<DynInstDataOpInterface>()) {
+      assert(tclOp.getTopModule(topLevelSymbols) == mod &&
+             "Referenced mod does does not match");
+      tclOps.push_back(tclOp);
+    }
   }
+
+  // Locations at the global scope are assumed to refer to the module without an
+  // instance.
+  for (auto tclOp : topLevel.getOps<DynInstDataOpInterface>()) {
+    Operation *mod = tclOp.getTopModule(topLevelSymbols);
+    assert(mod && "Must be able to resolve top module");
+    tclOpsForModInstance[mod][{}].push_back(tclOp);
+  }
+
   return success();
 }
 
@@ -102,6 +105,19 @@ struct TclOutputState {
 
   void emitPath(hw::GlobalRefOp ref, Optional<StringRef> subpath);
   void emitInnerRefPart(hw::InnerRefAttr innerRef);
+
+  /// Get the GlobalRefOp to which the given operation is pointing. Add it to
+  /// the set of used global refs.
+  GlobalRefOp getRefOp(DynInstDataOpInterface op) {
+    auto ref = dyn_cast_or_null<hw::GlobalRefOp>(
+        emitter.getDefinition(op.getGlobalRefSym()));
+    if (ref)
+      emitter.usedRef(ref);
+    else
+      op.emitOpError("could not find hw.globalRef named ")
+          << op.getGlobalRefSym();
+    return ref;
+  }
 };
 } // anonymous namespace
 
@@ -162,18 +178,12 @@ LogicalResult
 TclOutputState::emitLocationAssignment(DynInstDataOpInterface refOp,
                                        PhysLocationAttr loc,
                                        Optional<StringRef> subpath) {
-  auto ref = dyn_cast_or_null<hw::GlobalRefOp>(
-      emitter.getDefinition(refOp.getGlobalRefSym()));
-  if (!ref)
-    return refOp.emitOpError("could not find hw.globalRef named ")
-           << refOp.getGlobalRefSym();
-
   indent() << "set_location_assignment ";
   emit(loc);
 
   // To which entity does this apply?
   os << " -to $parent|";
-  emitPath(ref, subpath);
+  emitPath(getRefOp(refOp), subpath);
 
   return success();
 }
@@ -201,12 +211,7 @@ LogicalResult TclOutputState::emit(PDRegPhysLocationOp locs) {
 /// Emit tcl in the form of:
 /// "set_global_assignment -name NAME VALUE -to $parent|fooInst|entityName"
 LogicalResult TclOutputState::emit(DynamicInstanceVerbatimAttrOp attr) {
-
-  auto ref =
-      dyn_cast_or_null<hw::GlobalRefOp>(emitter.getDefinition(attr.refAttr()));
-  if (!ref)
-    return attr.emitOpError("could not find hw.globalRef named ")
-           << attr.refAttr();
+  GlobalRefOp ref = getRefOp(attr);
   indent() << "set_instance_assignment -name " << attr.name() << " "
            << attr.value();
 
@@ -223,11 +228,7 @@ LogicalResult TclOutputState::emit(DynamicInstanceVerbatimAttrOp attr) {
 /// set_instance_assignment -name CORE_ONLY_PLACE_REGION ON -to $parent|a|b|c
 /// set_instance_assignment -name REGION_NAME test_region -to $parent|a|b|c
 LogicalResult TclOutputState::emit(PDPhysRegionOp region) {
-  auto ref = dyn_cast_or_null<hw::GlobalRefOp>(
-      emitter.getDefinition(region.refAttr()));
-  if (!ref)
-    return region.emitOpError("could not find hw.globalRef named ")
-           << region.refAttr();
+  GlobalRefOp ref = getRefOp(region);
 
   auto physicalRegion = dyn_cast_or_null<DeclPhysicalRegionOp>(
       emitter.getDefinition(region.physRegionRefAttr()));
@@ -287,27 +288,37 @@ LogicalResult TclEmitter::emit(Operation *hwMod, StringRef outputFile) {
   std::string s;
   llvm::raw_string_ostream os(s);
   TclOutputState state(*this, os);
-  os << "proc {{" << state.symbolRefs.size() << "}}_config { parent } {\n";
-  state.symbolRefs.push_back(SymbolRefAttr::get(hwMod));
 
-  // Loop through the ops relevant to the specified root module.
-  LogicalResult ret = success();
-  for (Operation *tclOp : tclOpsForMod[hwMod]) {
-    LogicalResult rc =
-        TypeSwitch<Operation *, LogicalResult>(tclOp)
-            .Case([&](PDPhysLocationOp op) { return state.emit(op); })
-            .Case([&](PDRegPhysLocationOp op) { return state.emit(op); })
-            .Case([&](PDPhysRegionOp op) { return state.emit(op); })
-            .Case([&](DynamicInstanceVerbatimAttrOp op) {
-              return state.emit(op);
-            })
-            .Default([](Operation *op) {
-              return op->emitOpError("could not determine how to output tcl");
-            });
-    if (failed(rc))
-      ret = failure();
+  // Iterate through all the "instances" for 'hwMod' and produce a tcl proc for
+  // each one.
+  for (auto tclOpsForInstancesKV : tclOpsForModInstance[hwMod]) {
+    StringAttr instName = tclOpsForInstancesKV.first;
+    os << "proc {{" << state.symbolRefs.size() << "}}";
+    if (instName)
+      os << '_' << instName.getValue();
+    os << "_config { parent } {\n";
+    state.symbolRefs.push_back(SymbolRefAttr::get(hwMod));
+
+    // Loop through the ops relevant to the specified root module "instance".
+    LogicalResult ret = success();
+    auto &tclOpsForMod = tclOpsForInstancesKV.second;
+    for (Operation *tclOp : tclOpsForMod) {
+      LogicalResult rc =
+          TypeSwitch<Operation *, LogicalResult>(tclOp)
+              .Case([&](PDPhysLocationOp op) { return state.emit(op); })
+              .Case([&](PDRegPhysLocationOp op) { return state.emit(op); })
+              .Case([&](PDPhysRegionOp op) { return state.emit(op); })
+              .Case([&](DynamicInstanceVerbatimAttrOp op) {
+                return state.emit(op);
+              })
+              .Default([](Operation *op) {
+                return op->emitOpError("could not determine how to output tcl");
+              });
+      if (failed(rc))
+        ret = failure();
+    }
+    os << "}\n\n";
   }
-  os << "}\n\n";
 
   // Create a verbatim op containing the Tcl and symbol references.
   OpBuilder builder = OpBuilder::atBlockEnd(hwMod->getBlock());

lib/Dialect/MSFT/MSFTPasses.cpp

@@ -66,7 +66,8 @@ namespace {
 struct LowerInstancesPass : public LowerInstancesBase<LowerInstancesPass> {
   void runOnOperation() override;
 
-  LogicalResult lower(DynamicInstanceOp inst, OpBuilder &b);
+  LogicalResult lower(DynamicInstanceOp inst, InstanceHierarchyOp hier,
+                      OpBuilder &b);
 
   // Aggregation of the global ref attributes populated as a side-effect of the
   // conversion.
@@ -100,7 +101,9 @@ const SymbolCache &LowerInstancesPass::getSyms(MSFTModuleOp mod) {
   return syms;
 }
 
-LogicalResult LowerInstancesPass::lower(DynamicInstanceOp inst, OpBuilder &b) {
+LogicalResult LowerInstancesPass::lower(DynamicInstanceOp inst,
+                                        InstanceHierarchyOp hier,
+                                        OpBuilder &b) {
 
   hw::GlobalRefOp ref = nullptr;
 
@@ -155,11 +158,12 @@ LogicalResult LowerInstancesPass::lower(DynamicInstanceOp inst, OpBuilder &b) {
   }
 
   // Relocate all my children.
+  OpBuilder hierBlock(&hier.body().getBlocks().front().front());
   for (Operation &op : llvm::make_early_inc_range(inst.getOps())) {
     // Child instances should have been lowered already.
     assert(!isa<DynamicInstanceOp>(op));
     op.remove();
-    b.insert(&op);
+    hierBlock.insert(&op);
 
     // Assign a ref for ops which need it.
     if (auto specOp = dyn_cast<DynInstDataOpInterface>(op)) {
@@ -183,16 +187,16 @@ void LowerInstancesPass::runOnOperation() {
 
   // Find all of the InstanceHierarchyOps.
   for (Operation &op : llvm::make_early_inc_range(top.getOps())) {
-    if (!isa<InstanceHierarchyOp>(op))
+    auto instHierOp = dyn_cast<InstanceHierarchyOp>(op);
+    if (!instHierOp)
       continue;
     builder.setInsertionPoint(&op);
     // Walk the child dynamic instances in _post-order_ so we lower and delete
     // the children first.
-    op.walk<mlir::WalkOrder::PostOrder>([&](DynamicInstanceOp inst) {
-      if (failed(lower(inst, builder)))
+    instHierOp->walk<mlir::WalkOrder::PostOrder>([&](DynamicInstanceOp inst) {
+      if (failed(lower(inst, instHierOp, builder)))
         ++numFailed;
     });
-    op.erase();
   }
   if (numFailed)
     signalPassFailure();
@@ -423,7 +427,8 @@ void LowerToHWPass::runOnOperation() {
 
   // Then, convert the InstanceOps
   target.addDynamicallyLegalDialect<MSFTDialect>([](Operation *op) {
-    return isa<DynInstDataOpInterface, DeclPhysicalRegionOp>(op);
+    return isa<DynInstDataOpInterface, DeclPhysicalRegionOp,
+               InstanceHierarchyOp>(op);
   });
   RewritePatternSet instancePatterns(ctxt);
   instancePatterns.insert<InstanceOpLowering>(ctxt);
@@ -446,24 +451,15 @@ std::unique_ptr<Pass> createLowerToHWPass() {
 namespace {
 template <typename PhysOpTy>
 struct RemovePhysOpLowering : public OpConversionPattern<PhysOpTy> {
+  using OpConversionPattern<PhysOpTy>::OpConversionPattern;
   using OpAdaptor = typename OpConversionPattern<PhysOpTy>::OpAdaptor;
 
-  RemovePhysOpLowering(MLIRContext *ctxt, DenseSet<SymbolRefAttr> &refsUsed)
-      : OpConversionPattern<PhysOpTy>::OpConversionPattern(ctxt),
-        refsUsed(refsUsed) {}
-
   LogicalResult
   matchAndRewrite(PhysOpTy op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const final {
-    SymbolRefAttr refSym = op->template getAttrOfType<FlatSymbolRefAttr>("ref");
-    if (refSym)
-      refsUsed.insert(refSym);
     rewriter.eraseOp(op);
     return success();
   }
-
-private:
-  DenseSet<SymbolRefAttr> &refsUsed;
 };
 } // anonymous namespace
 
@@ -501,18 +497,17 @@ void ExportTclPass::runOnOperation() {
   target.addLegalDialect<sv::SVDialect>();
 
   RewritePatternSet patterns(ctxt);
-  DenseSet<SymbolRefAttr> refsUsed;
-  patterns.insert<RemovePhysOpLowering<PDPhysLocationOp>>(ctxt, refsUsed);
-  patterns.insert<RemovePhysOpLowering<PDRegPhysLocationOp>>(ctxt, refsUsed);
-  patterns.insert<RemovePhysOpLowering<PDPhysRegionOp>>(ctxt, refsUsed);
-  patterns.insert<RemovePhysOpLowering<DynamicInstanceVerbatimAttrOp>>(
-      ctxt, refsUsed);
+  patterns.insert<RemovePhysOpLowering<PDPhysLocationOp>>(ctxt);
+  patterns.insert<RemovePhysOpLowering<PDRegPhysLocationOp>>(ctxt);
+  patterns.insert<RemovePhysOpLowering<PDPhysRegionOp>>(ctxt);
+  patterns.insert<RemovePhysOpLowering<InstanceHierarchyOp>>(ctxt);
+  patterns.insert<RemovePhysOpLowering<DynamicInstanceVerbatimAttrOp>>(ctxt);
   patterns.insert<RemoveOpLowering<DeclPhysicalRegionOp>>(ctxt);
   if (failed(applyPartialConversion(top, target, std::move(patterns))))
     signalPassFailure();
 
   target.addDynamicallyLegalOp<hw::GlobalRefOp>([&](hw::GlobalRefOp ref) {
-    return !refsUsed.contains(SymbolRefAttr::get(ref));
+    return !emitter.getRefsUsed().contains(ref);
   });
   patterns.clear();
   patterns.insert<RemoveOpLowering<hw::GlobalRefOp>>(ctxt);

test/Dialect/MSFT/dynamic_instance.mlir

@@ -1,6 +1,6 @@
 // RUN: circt-opt %s -verify-diagnostics | circt-opt -verify-diagnostics
 // RUN: circt-opt %s --msft-lower-instances --verify-each | FileCheck %s
-// RUN: circt-opt %s --msft-lower-instances --lower-msft-to-hw --lower-seq-to-sv --msft-export-tcl=tops=shallow,deeper,reg --export-verilog | FileCheck %s --check-prefix=TCL
+// RUN: circt-opt %s --msft-lower-instances --lower-msft-to-hw --lower-seq-to-sv --msft-export-tcl=tops=shallow,deeper,reg --export-verilog -o %t.mlir | FileCheck %s --check-prefix=TCL
 
 msft.instance.hierarchy @deeper {
   msft.instance.dynamic @deeper::@branch {
@@ -25,13 +25,20 @@ msft.instance.hierarchy @shallow {
 // CHECK: hw.globalRef @instref_1 [#hw.innerNameRef<@shallow::@leaf>, #hw.innerNameRef<@leaf::@module>]
 // CHECK: msft.pd.location @instref_1 M20K x: 8 y: 19 n: 1 path : "|memBank2"
 
-msft.instance.hierarchy @reg {
+msft.instance.hierarchy @reg "foo" {
   msft.instance.dynamic @reg::@reg {
     msft.pd.reg_location i4 [*, <1,2,3>, <1,2,4>, <1,2,5>]
   }
 }
+msft.instance.hierarchy @reg "bar" {
+  msft.instance.dynamic @reg::@reg {
+    msft.pd.reg_location i4 [<3,4,5>, *, *, *]
+  }
+}
 // CHECK: hw.globalRef @instref_2 [#hw.innerNameRef<@reg::@reg>]
-// CHECK: msft.pd.reg_location ref @instref_2 i4 [*, <1, 2, 3>, <1, 2, 4>, <1, 2, 5>]
+// CHECK-DAG: msft.pd.reg_location ref @instref_2 i4 [*, <1, 2, 3>, <1, 2, 4>, <1, 2, 5>]
+// CHECK: hw.globalRef @instref_3 [#hw.innerNameRef<@reg::@reg>]
+// CHECK-DAG: msft.pd.reg_location ref @instref_3 i4 [<3, 4, 5>, *, *, *]
 
 
 
@@ -71,11 +78,14 @@ msft.module @deeper {} () -> () {
   msft.output
 }
 
-// TCL-LABEL: proc reg_0_config
 msft.module @reg {} (%input : i4, %clk : i1) -> () {
   %reg = seq.compreg sym @reg %input, %clk  : i4
-  // TCL: set_location_assignment FF_X1_Y2_N3 -to $parent|reg_1[1]
-  // TCL: set_location_assignment FF_X1_Y2_N4 -to $parent|reg_1[2]
-  // TCL: set_location_assignment FF_X1_Y2_N5 -to $parent|reg_1[3]
   msft.output
 }
+// TCL-LABEL: proc reg_0_foo_config
+// TCL: set_location_assignment FF_X1_Y2_N3 -to $parent|reg_1[1]
+// TCL: set_location_assignment FF_X1_Y2_N4 -to $parent|reg_1[2]
+// TCL: set_location_assignment FF_X1_Y2_N5 -to $parent|reg_1[3]
+
+// TCL-LABEL: proc reg_0_bar_config
+// TCL: set_location_assignment FF_X3_Y4_N5 -to $parent|reg_1[0]