[NFC] Move SymCache to its own header

include/circt/Dialect/HW/HWOps.h

@@ -170,95 +170,6 @@ StringAttr getArgSym(Operation *op, unsigned i);
 /// argument.
 StringAttr getResultSym(Operation *op, unsigned i);
 
-/// This stores lookup tables to make manipulating and working with the IR more
-/// efficient.  There are two phases to this object: the "building" phase in
-/// which it is "write only" and then the "using" phase which is read-only (and
-/// thus can be used by multiple threads).  The
-/// "freeze" method transitions between the two states.
-class SymbolCache {
-public:
-  class Item {
-  public:
-    Item(Operation *op) : op(op), port(~0ULL) {}
-    Item(Operation *op, size_t port) : op(op), port(port) {}
-    bool hasPort() const { return port != ~0ULL; }
-    Operation *getOp() const { return op; }
-    size_t getPort() const { return port; }
-
-  private:
-    Operation *op;
-    size_t port;
-  };
-
-  /// In the building phase, add symbols.
-  void addDefinition(StringAttr symbol, Operation *op) {
-    assert(!isFrozen && "cannot mutate a frozen cache");
-    symbolCache.try_emplace(symbol.getValue(), op, ~0ULL);
-  }
-
-  // Add inner names, which might be ports
-  void addDefinition(StringAttr symbol, StringRef name, Operation *op,
-                     size_t port = ~0ULL) {
-    assert(!isFrozen && "cannot mutate a frozen cache");
-    auto key = mkInnerKey(symbol.getValue(), name);
-    symbolCache.try_emplace(StringRef(key.data(), key.size()), op, port);
-  }
-
-  /// Mark the cache as frozen, which allows it to be shared across threads.
-  void freeze() { isFrozen = true; }
-
-  Operation *getDefinition(StringRef symbol) const {
-    assert(isFrozen && "cannot read from this cache until it is frozen");
-    auto it = symbolCache.find(symbol);
-    if (it == symbolCache.end())
-      return nullptr;
-    assert(!it->second.hasPort() && "Module names should never be ports");
-    return it->second.getOp();
-  }
-
-  Operation *getDefinition(StringAttr symbol) const {
-    return getDefinition(symbol.getValue());
-  }
-
-  Operation *getDefinition(FlatSymbolRefAttr symbol) const {
-    return getDefinition(symbol.getValue());
-  }
-
-  Item getDefinition(StringRef symbol, StringRef name) const {
-    assert(isFrozen && "cannot read from this cache until it is frozen");
-    auto key = mkInnerKey(symbol, name);
-    auto it = symbolCache.find(StringRef(key.data(), key.size()));
-    return it == symbolCache.end() ? Item{nullptr, ~0ULL} : it->second;
-  }
-
-  Item getDefinition(StringAttr symbol, StringAttr name) const {
-    return getDefinition(symbol.getValue(), name.getValue());
-  }
-
-private:
-  bool isFrozen = false;
-
-  /// This stores a lookup table from symbol attribute to the operation
-  /// (hw.module, hw.instance, etc) that defines it.
-  /// TODO: It is super annoying that symbols are *defined* as StringAttr, but
-  /// are then referenced as FlatSymbolRefAttr.  Why can't we have nice
-  /// pointer uniqued things?? :-(
-  llvm::StringMap<Item> symbolCache;
-
-  // Construct a string key with embedded null.  StringMapImpl::FindKey uses
-  // explicit lengths and stores keylength, rather than relying on null
-  // characters.
-  SmallVector<char> mkInnerKey(StringRef mod, StringRef name) const {
-    assert(!mod.contains(0) && !name.contains(0) &&
-           "Null character in identifier");
-    SmallVector<char> key;
-    key.append(mod.begin(), mod.end());
-    key.push_back(0);
-    key.append(name.begin(), name.end());
-    return key;
-  }
-};
-
 } // namespace hw
 } // namespace circt
 

include/circt/Dialect/HW/HWSymCache.h

@@ -0,0 +1,108 @@
+//===- HWSymCache.h - Declare Symbol Cache ---------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file declares a Symbol Cache.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef CIRCT_DIALECT_HW_SYMCACHE_H
+#define CIRCT_DIALECT_HW_SYMCACHE_H
+
+#include "circt/Dialect/HW/HWAttributes.h"
+
+namespace circt {
+namespace hw {
+
+/// This stores lookup tables to make manipulating and working with the IR more
+/// efficient.  There are two phases to this object: the "building" phase in
+/// which it is "write only" and then the "using" phase which is read-only (and
+/// thus can be used by multiple threads).  The
+/// "freeze" method transitions between the two states.
+class SymbolCache {
+public:
+  class Item {
+  public:
+    Item(mlir::Operation *op) : op(op), port(~0ULL) {}
+    Item(mlir::Operation *op, size_t port) : op(op), port(port) {}
+    bool hasPort() const { return port != ~0ULL; }
+    mlir::Operation *getOp() const { return op; }
+    size_t getPort() const { return port; }
+
+  private:
+    mlir::Operation *op;
+    size_t port;
+  };
+
+  /// In the building phase, add symbols.
+  void addDefinition(mlir::StringAttr symbol, mlir::Operation *op) {
+    assert(!isFrozen && "cannot mutate a frozen cache");
+    symbolCache.try_emplace(symbol, op, ~0ULL);
+  }
+
+  // Add inner names, which might be ports
+  void addDefinition(mlir::StringAttr modSymbol, mlir::StringAttr name,
+                     mlir::Operation *op, size_t port = ~0ULL) {
+    assert(!isFrozen && "cannot mutate a frozen cache");
+    auto key = InnerRefAttr::get(modSymbol.getContext(), modSymbol, name);
+    symbolCache.try_emplace(key, op, port);
+  }
+
+  // Add inner names, which might be ports
+  void addDefinition(InnerRefAttr name, mlir::Operation *op,
+                     size_t port = ~0ULL) {
+    assert(!isFrozen && "cannot mutate a frozen cache");
+    symbolCache.try_emplace(name, op, port);
+  }
+
+  /// Mark the cache as frozen, which allows it to be shared across threads.
+  void freeze() { isFrozen = true; }
+
+  mlir::Operation *getDefinition(mlir::StringAttr symbol) const {
+    return lookup(symbol);
+  }
+
+  mlir::Operation *getDefinition(mlir::FlatSymbolRefAttr symbol) const {
+    return lookup(symbol.getAttr());
+  }
+
+  Item getDefinition(mlir::StringAttr modSymbol, mlir::StringAttr name) const {
+    return lookup(InnerRefAttr::get(modSymbol.getContext(), modSymbol, name));
+  }
+
+  Item getDefinition(InnerRefAttr name) const { return lookup(name); }
+
+private:
+  Item lookup(InnerRefAttr attr) const {
+    assert(isFrozen && "cannot read from this cache until it is frozen");
+    auto it = symbolCache.find(attr);
+    return it == symbolCache.end() ? Item{nullptr, ~0ULL} : it->second;
+  }
+
+  mlir::Operation *lookup(mlir::StringAttr attr) const {
+    assert(isFrozen && "cannot read from this cache until it is frozen");
+    auto it = symbolCache.find(attr);
+    if (it == symbolCache.end())
+      return nullptr;
+    assert(!it->second.hasPort() && "Module names should never be ports");
+    return it->second.getOp();
+  }
+
+  bool isFrozen = false;
+
+  /// This stores a lookup table from symbol attribute to the operation
+  /// (hw.module, hw.instance, etc) that defines it.
+  /// TODO: It is super annoying that symbols are *defined* as StringAttr, but
+  /// are then referenced as FlatSymbolRefAttr.  Why can't we have nice
+  /// pointer uniqued things?? :-(
+  llvm::DenseMap<mlir::Attribute, Item> symbolCache;
+};
+
+} // namespace hw
+} // namespace circt
+
+#endif // CIRCT_DIALECT_HW_SYMCACHE_H

lib/CAPI/Dialect/MSFT.cpp

@@ -3,6 +3,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "circt-c/Dialect/MSFT.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/MSFT/DeviceDB.h"
 #include "circt/Dialect/MSFT/ExportTcl.h"
 #include "circt/Dialect/MSFT/MSFTAttributes.h"

lib/Conversion/ExportVerilog/ExportVerilog.cpp

@@ -4120,14 +4120,13 @@ void SharedEmitterState::gatherFiles(bool separateModules) {
       // Populate the symbolCache with all operations that can define a symbol.
       if (auto name = op->getAttrOfType<StringAttr>(
               hw::InnerName::getInnerNameAttrName()))
-        symbolCache.addDefinition(moduleOp.getNameAttr(), name.getValue(), op);
+        symbolCache.addDefinition(moduleOp.getNameAttr(), name, op);
       // HACK: This is to make interface-related operations work as they are at
       // the moment, with names being stored in `sym_name` instead of
       // `inner_sym`.
       if (auto instOp = dyn_cast<InterfaceInstanceOp>(op))
         if (auto attr = instOp.sym_nameAttr())
-          symbolCache.addDefinition(moduleOp.getNameAttr(), attr.getValue(),
-                                    op);
+          symbolCache.addDefinition(moduleOp.getNameAttr(), attr, op);
       if (isa<BindOp>(op))
         modulesContainingBinds.insert(moduleOp);
     });
@@ -4138,12 +4137,12 @@ void SharedEmitterState::gatherFiles(bool separateModules) {
     for (size_t p = 0; p != numArgs; ++p)
       for (NamedAttribute argAttr : moduleOp.getArgAttrs(p))
         if (auto sym = argAttr.getValue().dyn_cast<FlatSymbolRefAttr>())
-          symbolCache.addDefinition(moduleOp.getNameAttr(), sym.getValue(),
+          symbolCache.addDefinition(moduleOp.getNameAttr(), sym.getAttr(),
                                     moduleOp, p);
     for (size_t p = 0, e = moduleOp.getNumResults(); p != e; ++p)
       for (NamedAttribute resultAttr : moduleOp.getResultAttrs(p))
         if (auto sym = resultAttr.getValue().dyn_cast<FlatSymbolRefAttr>())
-          symbolCache.addDefinition(moduleOp.getNameAttr(), sym.getValue(),
+          symbolCache.addDefinition(moduleOp.getNameAttr(), sym.getAttr(),
                                     moduleOp, p + numArgs);
   };
 

lib/Conversion/ExportVerilog/ExportVerilogInternals.h

@@ -10,6 +10,7 @@
 #define CONVERSION_EXPORTVERILOG_EXPORTVERILOGINTERNAL_H
 
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/SV/SVOps.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallPtrSet.h"

lib/Dialect/FIRRTL/Transforms/GrandCentralTaps.cpp

@@ -21,6 +21,7 @@
 #include "circt/Dialect/FIRRTL/Passes.h"
 #include "circt/Dialect/HW/HWAttributes.h"
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/SV/SVOps.h"
 #include "mlir/IR/ImplicitLocOpBuilder.h"
 #include "llvm/ADT/STLExtras.h"

lib/Dialect/HW/HWOps.cpp

@@ -14,6 +14,7 @@
 #include "circt/Dialect/Comb/CombOps.h"
 #include "circt/Dialect/FIRRTL/FIRRTLOps.h"
 #include "circt/Dialect/HW/HWAttributes.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/HW/HWVisitors.h"
 #include "circt/Dialect/HW/ModuleImplementation.h"
 #include "mlir/IR/Builders.h"

lib/Dialect/HW/HWTypes.cpp

@@ -14,6 +14,7 @@
 #include "circt/Dialect/HW/HWAttributes.h"
 #include "circt/Dialect/HW/HWDialect.h"
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Support/LLVM.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinTypes.h"

lib/Dialect/MSFT/ExportQuartusTcl.cpp

@@ -12,6 +12,7 @@
 
 #include "circt/Dialect/HW/HWAttributes.h"
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/HW/HWTypes.h"
 #include "circt/Dialect/MSFT/DeviceDB.h"
 #include "circt/Dialect/MSFT/ExportTcl.h"

lib/Dialect/MSFT/MSFTPasses.cpp

@@ -8,6 +8,7 @@
 
 #include "circt/Dialect/HW/HWAttributes.h"
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/HW/HWTypes.h"
 #include "circt/Dialect/MSFT/ExportTcl.h"
 #include "circt/Dialect/MSFT/MSFTDialect.h"

lib/Dialect/SV/SVOps.cpp

@@ -14,6 +14,7 @@
 #include "circt/Dialect/Comb/CombOps.h"
 #include "circt/Dialect/HW/HWAttributes.h"
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/HW/HWTypes.h"
 #include "mlir/IR/Builders.h"
 #include "mlir/IR/BuiltinTypes.h"

lib/Dialect/SV/Transforms/SVExtractTestCode.cpp

@@ -16,6 +16,7 @@
 #include "PassDetail.h"
 #include "circt/Dialect/HW/HWAttributes.h"
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/Dialect/SV/SVPasses.h"
 #include "mlir/IR/BlockAndValueMapping.h"
 #include "mlir/IR/Builders.h"

tools/circt-translate/circt-translate.cpp

@@ -11,6 +11,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "circt/Dialect/HW/HWSymCache.h"
 #include "circt/InitAllTranslations.h"
 #include "mlir/Support/LogicalResult.h"
 #include "mlir/Translation.h"