[SSP] Reimplement auxiliary dependences without symbols

include/circt/Dialect/SSP/SSPAttributes.td

@@ -25,7 +25,7 @@ def DependenceAttr : AttrDef<SSPDialect, "Dependence"> {
   }];
 
   let parameters = (ins "unsigned":$operandIdx,
-                        OptionalParameter<"::mlir::FlatSymbolRefAttr">:$sourceRef,
+                        OptionalParameter<"::mlir::StringAttr">:$sourceRef,
                         OptionalParameter<"::mlir::ArrayAttr">:$properties);
 
   let mnemonic = "dependence";

include/circt/Dialect/SSP/SSPOps.td

@@ -170,7 +170,7 @@ def ResourceLibraryOp : SSPOp<"resource",
 
 def DependenceGraphOp : SSPOp<"graph",
     [HasOnlyGraphRegion, NoRegionArguments,
-     SingleBlock, NoTerminator, OpAsmOpInterface, SymbolTable,
+     SingleBlock, NoTerminator, OpAsmOpInterface,
      HasParent<"InstanceOp">]> {
   let summary = "Container for (scheduling) operations.";
   let description = [{
@@ -181,6 +181,8 @@ def DependenceGraphOp : SSPOp<"graph",
   let assemblyFormat = "$body attr-dict";
   let regions = (region SizedRegion<1>:$body);
 
+  let hasRegionVerifier = true;
+
   let extraClassDeclaration = [{
     // OpAsmOpInterface
     static ::llvm::StringRef getDefaultDialect() { return "ssp"; }
@@ -189,6 +191,9 @@ def DependenceGraphOp : SSPOp<"graph",
     ::mlir::Block *getBodyBlock() {
       return &getBody().getBlocks().front();
     }
+
+    // Retrieve `OperationOp`s by name (for auxiliary dependences).
+    ::circt::ssp::OperationOp lookupNamedOperation(::mlir::StringRef name);
   }];
 
   let skipDefaultBuilders = true;
@@ -278,7 +283,7 @@ def OperationOp : SSPOp<"operation",
   }];
 
   let arguments = (ins Variadic<NoneType>:$operands,
-                       OptionalAttr<SymbolNameAttr>:$sym_name,
+                       OptionalAttr<StrAttr>:$name,
                        OptionalAttr<DependenceArrayAttr>:$dependences,
                        OptionalAttr<ArrayAttr>:$sspProperties);
   let results = (outs Variadic<NoneType>:$results);
@@ -287,9 +292,6 @@ def OperationOp : SSPOp<"operation",
   let hasVerifier = true;
 
   let extraClassDeclaration = [{
-    // SymbolOpInterface
-    static bool isOptionalSymbol() { return true; }
-
     // Find the attribute modeling the `linkedOperatorType` property
     ::circt::ssp::LinkedOperatorTypeAttr getLinkedOperatorTypeAttr();
 
@@ -301,13 +303,13 @@ def OperationOp : SSPOp<"operation",
   let builders = [
     OpBuilder<(ins "unsigned":$numResults,
                    "::mlir::ValueRange":$operands,
-                   CArg<"::mlir::StringAttr", "::mlir::StringAttr()">:$sym_name,
+                   CArg<"::mlir::StringAttr", "::mlir::StringAttr()">:$name,
                    CArg<"::mlir::ArrayAttr", "::mlir::ArrayAttr()">:$dependences,
                    CArg<"::mlir::ArrayAttr", "::mlir::ArrayAttr()">:$sspProperties), [{
       $_state.addTypes(::llvm::SmallVector<::mlir::Type>(numResults, $_builder.getNoneType()));
       $_state.addOperands(operands);
-      if (sym_name)
-        $_state.addAttribute(::mlir::SymbolTable::getSymbolAttrName(), sym_name);
+      if (name)
+        $_state.addAttribute($_builder.getStringAttr("name"), name);
       if (dependences)
         $_state.addAttribute($_builder.getStringAttr("dependences"), dependences);
       if (sspProperties)

include/circt/Dialect/SSP/Utilities.h

@@ -224,14 +224,20 @@ ProblemT loadProblem(InstanceOp instOp,
   if (auto rsrcLibName = rsrcLibraryOp.getSymNameAttr())
     prob.setRsrcLibraryName(rsrcLibName);
 
+  // Build ad-hoc symbol table to resolve auxiliary dependences.
+  SmallDenseMap<StringAttr, OperationOp> namedOps;
+
   // Register all operations first, in order to retain their original order.
   auto graphOp = instOp.getDependenceGraph();
   graphOp.walk([&](OperationOp opOp) {
     prob.insertOperation(opOp);
     loadOperationProperties<ProblemT, OperationPropertyTs...>(
         prob, opOp, opOp.getSspPropertiesAttr());
-    if (auto opName = opOp.getSymNameAttr())
+    if (StringAttr opName = opOp.getNameAttr()) {
       prob.setOperationName(opOp, opName);
+      [[maybe_unused]] auto [it, ins] = namedOps.try_emplace(opName, opOp);
+      assert(ins && "Non-unique operation name detected");
+    }
 
     // Nothing else to check if no linked operator type is set for `opOp`,
     // because the operation doesn't carry a `LinkedOperatorTypeAttr`, or that
@@ -315,8 +321,8 @@ ProblemT loadProblem(InstanceOp instOp,
 
     for (auto depAttr : depsAttr.getAsRange<DependenceAttr>()) {
       Dependence dep;
-      if (FlatSymbolRefAttr sourceRef = depAttr.getSourceRef()) {
-        Operation *sourceOp = SymbolTable::lookupSymbolIn(graphOp, sourceRef);
+      if (StringAttr sourceRef = depAttr.getSourceRef()) {
+        OperationOp sourceOp = namedOps.lookup(sourceRef);
         assert(sourceOp);
         dep = Dependence(sourceOp, opOp);
         LogicalResult res = prob.insertDependence(dep);
@@ -507,17 +513,16 @@ saveProblem(ProblemT &prob, std::tuple<OperationPropertyTs...> opProps,
                                                                       b);
       if (dep.isDefUse() && depProps) {
         auto depAttr = b.getAttr<DependenceAttr>(*dep.getDestinationIndex(),
-                                                 FlatSymbolRefAttr(), depProps);
+                                                 StringAttr(), depProps);
         depAttrs.push_back(depAttr);
         continue;
       }
 
       if (!dep.isAuxiliary())
         continue;
 
-      auto sourceOpName = opNames.lookup(dep.getSource());
-      assert(sourceOpName);
-      auto sourceRef = b.getAttr<FlatSymbolRefAttr>(sourceOpName);
+      auto sourceRef = opNames.lookup(dep.getSource());
+      assert(sourceRef);
       auto depAttr =
           b.getAttr<DependenceAttr>(auxOperandIdx, sourceRef, depProps);
       depAttrs.push_back(depAttr);

lib/Dialect/SSP/SSPOps.cpp

@@ -53,6 +53,50 @@ DependenceGraphOp InstanceOp::getDependenceGraph() {
   return *getOps<DependenceGraphOp>().begin();
 }
 
+//===----------------------------------------------------------------------===//
+// DependenceGraphOp
+//===----------------------------------------------------------------------===//
+
+LogicalResult DependenceGraphOp::verifyRegions() {
+  SmallDenseMap<StringAttr, OperationOp> namedOps;
+
+  // Check uniqueness of operation names.
+  for (auto opOp : getOps<OperationOp>()) {
+    if (StringAttr name = opOp.getNameAttr()) {
+      [[maybe_unused]] auto [it, ins] = namedOps.try_emplace(name, opOp);
+      if (!ins)
+        return emitError("Contains multiple operations named @")
+               << name.getValue();
+    }
+  }
+
+  // Check auxiliary dependences within this graph.
+  for (auto opOp : getOps<OperationOp>()) {
+    if (ArrayAttr dependences = opOp.getDependencesAttr()) {
+      for (auto dep : dependences.getAsRange<DependenceAttr>()) {
+        StringAttr sourceRef = dep.getSourceRef();
+        if (!sourceRef)
+          continue;
+
+        if (!namedOps.contains(sourceRef))
+          return opOp->emitError("Auxiliary dependence references invalid "
+                                 "source operation: @")
+                 << sourceRef.getValue();
+      }
+    }
+  }
+  return success();
+}
+
+OperationOp DependenceGraphOp::lookupNamedOperation(StringRef name) {
+  auto opOps = getOps<OperationOp>();
+  auto it = find_if(opOps, [&](OperationOp opOp) {
+    StringAttr nameAttr = opOp.getNameAttr();
+    return nameAttr && nameAttr.getValue() == name;
+  });
+  return it != opOps.end() ? *it : OperationOp{};
+}
+
 //===----------------------------------------------------------------------===//
 // OperationOp
 //===----------------------------------------------------------------------===//
@@ -78,23 +122,19 @@ ParseResult OperationOp::parse(OpAsmParser &parser, OperationState &result) {
 
   // (Scheduling) operation's name
   StringAttr opName;
-  (void)parser.parseOptionalSymbolName(opName, SymbolTable::getSymbolAttrName(),
-                                       result.attributes);
+  (void)parser.parseOptionalSymbolName(opName, "name", result.attributes);
 
   // Dependences
   SmallVector<OpAsmParser::UnresolvedOperand> unresolvedOperands;
   SmallVector<Attribute> dependences;
   unsigned operandIdx = 0;
   auto parseDependenceSourceWithAttrDict = [&]() -> ParseResult {
     llvm::SMLoc loc = parser.getCurrentLocation();
-    FlatSymbolRefAttr sourceRef;
+    StringAttr sourceRef;
     ArrayAttr properties;
 
-    // Try to parse either symbol reference...
-    auto parseSymbolResult = parser.parseOptionalAttribute(sourceRef);
-    if (parseSymbolResult.has_value())
-      assert(succeeded(*parseSymbolResult));
-    else {
+    // Try to parse either a reference to another op's @name...
+    if (parser.parseOptionalSymbolName(sourceRef)) {
       // ...or an SSA operand.
       OpAsmParser::UnresolvedOperand operand;
       if (parser.parseOperand(operand))
@@ -183,9 +223,9 @@ void OperationOp::print(OpAsmPrinter &p) {
   p << '>';
 
   // (Scheduling) operation's name
-  if (StringAttr symName = getSymNameAttr()) {
+  if (StringAttr name = getNameAttr()) {
     p << ' ';
-    p.printSymbolName(symName);
+    p.printSymbolName(name);
   }
 
   // Dependences = SSA operands + other OperationOps via symbol references.
@@ -214,7 +254,7 @@ void OperationOp::print(OpAsmPrinter &p) {
     if (!defUseDeps.empty())
       p << ", ";
     llvm::interleaveComma(auxDeps, p, [&](DependenceAttr dep) {
-      p.printAttribute(dep.getSourceRef());
+      p.printSymbolName(dep.getSourceRef());
       if (ArrayAttr depProps = dep.getProperties()) {
         p << ' ';
         printPropertyArray(depProps, p);
@@ -246,7 +286,7 @@ void OperationOp::print(OpAsmPrinter &p) {
 
   // Default attr-dict
   SmallVector<StringRef> elidedAttrs = {
-      SymbolTable::getSymbolAttrName(),
+      OperationOp::getNameAttrName().getValue(),
       OperationOp::getDependencesAttrName().getValue(),
       OperationOp::getSspPropertiesAttrName().getValue()};
   p.printOptionalAttrDict((*this)->getAttrs(), elidedAttrs);
@@ -261,7 +301,7 @@ LogicalResult OperationOp::verify() {
   int lastIdx = -1;
   for (auto dep : dependences.getAsRange<DependenceAttr>()) {
     int idx = dep.getOperandIdx();
-    FlatSymbolRefAttr sourceRef = dep.getSourceRef();
+    StringAttr sourceRef = dep.getSourceRef();
 
     if (!sourceRef) {
       // Def-use deps use the index to refer to one of the SSA operands.
@@ -277,7 +317,8 @@ LogicalResult OperationOp::verify() {
       // Auxiliary deps are expected to follow the def-use deps (if present),
       // and hence use indices >= #operands.
       if (idx < nOperands)
-        return emitError() << "Auxiliary dependence from " << sourceRef
+        return emitError() << "Auxiliary dependence from @"
+                           << sourceRef.getValue()
                            << " is interleaved with SSA operands";
 
       // Indices shall be consecutive (special case: the first aux dep)
@@ -295,23 +336,6 @@ LogicalResult
 OperationOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
   auto instanceOp = (*this)->getParentOfType<InstanceOp>();
   auto libraryOp = instanceOp.getOperatorLibrary();
-  auto graphOp = instanceOp.getDependenceGraph();
-
-  // Verify that all auxiliary dependences reference valid named operations
-  // inside the dependence graph.
-  if (ArrayAttr dependences = getDependencesAttr())
-    for (auto dep : dependences.getAsRange<DependenceAttr>()) {
-      FlatSymbolRefAttr sourceRef = dep.getSourceRef();
-      if (!sourceRef)
-        continue;
-
-      Operation *sourceOp = symbolTable.lookupSymbolIn(graphOp, sourceRef);
-      if (!sourceOp || !isa<OperationOp>(sourceOp)) {
-        return emitError(
-                   "Auxiliary dependence references invalid source operation: ")
-               << sourceRef;
-      }
-    }
 
   // If a linkedOperatorType property is present, verify that it references a
   // valid operator type.

lib/Dialect/SSP/Transforms/Schedule.cpp

@@ -51,7 +51,7 @@ static OperationOp getLastOp(InstanceOp instOp, StringRef options) {
   auto graphOp = instOp.getDependenceGraph();
   if (lastOpName.empty() && !graphOp.getBodyBlock()->empty())
     return cast<OperationOp>(graphOp.getBodyBlock()->back());
-  return graphOp.lookupSymbol<OperationOp>(lastOpName);
+  return graphOp.lookupNamedOperation(lastOpName);
 }
 
 // Determine desired cycle time (only relevant for `ChainingProblem` instances).

test/Dialect/SSP/errors.mlir

@@ -50,7 +50,7 @@ ssp.instance @deps_aux_not_consecutive of "Problem" {
   graph {
     operation<> @Op()
     // expected-error @+1 {{Auxiliary operand indices in dependence attribute are not consecutive}}
-    "ssp.operation"() {dependences = [#ssp.dependence<1, @Op>]} : () -> ()
+    "ssp.operation"() {dependences = [#ssp.dependence<1, "Op">]} : () -> ()
   }
 }
 
@@ -84,3 +84,14 @@ ssp.instance @standalone_opr_invalid of "Problem" {
     operation<@standalone::@InvalidOpr>()
   }
 }
+
+// -----
+
+ssp.instance @duplicate_op_names of "Problem" {
+  library {}
+  // expected-error @+1 {{Contains multiple operations named @Op}}
+  graph {
+    operation<> @Op()
+    operation<> @Op()
+  }
+}