[FIRRTL] Domain inference

include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td

@@ -119,17 +119,27 @@ def InstanceOp : HardwareDeclOp<"instance", [
            CArg<"hw::InnerSymAttr", "hw::InnerSymAttr()">:$innerSym)>];
 
   let extraClassDeclaration = [{
+    /// Return the number of ports for this instance.
+    size_t getNumPorts() {
+      return getNumResults();
+    }
+
     /// Return the port direction for the specified result number.
     Direction getPortDirection(size_t resultNo) {
       return direction::get(getPortDirections()[resultNo]);
     }
 
     /// Return the port name for the specified result number.
-    StringAttr getPortName(size_t resultNo) {
+    StringAttr getPortNameAttr(size_t resultNo) {
       return cast<StringAttr>(getPortNames()[resultNo]);
     }
-    StringRef getPortNameStr(size_t resultNo) {
-      return getPortName(resultNo).getValue();
+
+    StringRef getPortName(size_t resultNo) {
+      return getPortNameAttr(resultNo).getValue();
+    }
+
+    Location getPortLocation(size_t) {
+      return getLoc();
     }
 
     /// Hooks for port annotations.
@@ -208,15 +218,29 @@ def InstanceChoiceOp : HardwareDeclOp<"instance_choice", [
   let hasCustomAssemblyFormat = 1;
 
   let extraClassDeclaration = [{
+    /// Return the number of ports for this instance.
+    size_t getNumPorts() {
+      return getNumResults();
+    }
+
     /// Return the port direction for the specified result number.
     Direction getPortDirection(size_t resultNo) {
       return direction::get(getPortDirections()[resultNo]);
     }
 
     /// Return the port name for the specified result number.
-    StringAttr getPortName(size_t resultNo) {
+    StringAttr getPortNameAttr(size_t resultNo) {
       return cast<StringAttr>(getPortNames()[resultNo]);
     }
+
+    StringRef getPortName(size_t resultNo) {
+      return getPortNameAttr(resultNo).getValue();
+    }
+
+    Location getPortLocation(size_t resultNo) {
+      return getLoc();
+    }
+
     /// Return the default target attribute.
     FlatSymbolRefAttr getDefaultTargetAttr() {
       return llvm::cast<FlatSymbolRefAttr>(getModuleNamesAttr()[0]);
@@ -313,9 +337,10 @@ def MemOp : HardwareDeclOp<"mem", [DeclareOpInterfaceMethods<CombDataflow>]> {
     size_t getMaskBits();
 
     /// Return the port name for the specified result number.
-    StringAttr getPortName(size_t resultNo);
-    StringRef getPortNameStr(size_t resultNo) {
-      return getPortName(resultNo).getValue();
+    StringAttr getPortNameAttr(size_t resultNo);
+
+    StringRef getPortName(size_t resultNo) {
+      return getPortNameAttr(resultNo).getValue();
     }
 
     /// Return the port type for the specified result number.

include/circt/Dialect/FIRRTL/FIRRTLOpInterfaces.h

@@ -85,6 +85,28 @@ struct PortInfo {
         annotations(annos), domains(domains) {}
 };
 
+inline bool operator==(const PortInfo &lhs, const PortInfo &rhs) {
+  if (lhs.name != rhs.name)
+    return false;
+  if (lhs.type != rhs.type)
+    return false;
+  if (lhs.direction != rhs.direction)
+    return false;
+  if (lhs.sym != rhs.sym)
+    return false;
+  if (lhs.loc != rhs.loc)
+    return false;
+  if (lhs.annotations != rhs.annotations)
+    return false;
+  if (lhs.domains != rhs.domains)
+    return false;
+  return true;
+}
+
+inline bool operator!=(const PortInfo &lhs, const PortInfo &rhs) {
+  return !(lhs == rhs);
+}
+
 enum class ConnectBehaviorKind {
   /// Classic FIRRTL connections: last connect 'wins' across paths;
   /// conditionally applied under 'when'.

include/circt/Dialect/FIRRTL/Passes.td

@@ -909,6 +909,19 @@ def CheckLayers : Pass<"firrtl-check-layers", "firrtl::CircuitOp"> {
   }];
 }
 
+def InferDomains : Pass<"firrtl-infer-domains", "firrtl::CircuitOp"> {
+  let summary = "Infer and type check all firrtl domains";
+  let description = [{
+    This pass does domain inference on a FIRRTL circuit.  The end result of this
+    is either a corrrctly domain-checked FIRRTL circuit or failure with verbose
+    error messages indicating why the FIRRTL circuit has illegal domain
+    constructs.
+
+    E.g., this pass can be used to check for illegal clock-domain-crossings if
+    clock domains are specified for signals in the design.
+  }];
+}
+
 def LowerDomains : Pass<"firrtl-lower-domains", "firrtl::CircuitOp"> {
   let summary = "lower domain information to properties";
   let description = [{

include/circt/Firtool/Firtool.h

@@ -144,6 +144,8 @@ class FirtoolOptions {
 
   bool getEmitAllBindFiles() const { return emitAllBindFiles; }
 
+  bool shouldInferDomains() const { return inferDomains; }
+
   // Setters, used by the CAPI
   FirtoolOptions &setOutputFilename(StringRef name) {
     outputFilename = name;
@@ -393,6 +395,11 @@ class FirtoolOptions {
     return *this;
   }
 
+  FirtoolOptions &setInferDomains(bool value) {
+    inferDomains = value;
+    return *this;
+  }
+
 private:
   std::string outputFilename;
 
@@ -447,6 +454,7 @@ class FirtoolOptions {
   bool lintStaticAsserts;
   bool lintXmrsInDesign;
   bool emitAllBindFiles;
+  bool inferDomains;
 };
 
 void registerFirtoolCLOptions();

include/circt/Support/InstanceGraph.h

@@ -59,6 +59,11 @@ class InstanceGraphNode;
 class InstanceRecord
     : public llvm::ilist_node_with_parent<InstanceRecord, InstanceGraphNode> {
 public:
+  /// Get the op that this is tracking.
+  Operation *getOperation() {
+    return instance.getOperation();
+  }
+
   /// Get the instance-like op that this is tracking.
   template <typename TTarget = InstanceOpInterface>
   auto getInstance() {
@@ -113,6 +118,8 @@ class InstanceGraphNode : public llvm::ilist_node<InstanceGraphNode> {
 public:
   InstanceGraphNode() : module(nullptr) {}
 
+  Operation *getOperation() { return module.getOperation(); }
+
   /// Get the module that this node is tracking.
   template <typename TTarget = ModuleOpInterface>
   auto getModule() {

lib/Dialect/FIRRTL/Export/FIREmitter.cpp

@@ -1126,7 +1126,7 @@ void Emitter::emitStatement(InstanceOp op) {
   portName.push_back('.');
   unsigned baseLen = portName.size();
   for (unsigned i = 0, e = op.getNumResults(); i < e; ++i) {
-    portName.append(legalize(op.getPortName(i)));
+    portName.append(legalize(op.getPortNameAttr(i)));
     addValueName(op.getResult(i), portName);
     portName.resize(baseLen);
   }
@@ -1153,7 +1153,7 @@ void Emitter::emitStatement(InstanceChoiceOp op) {
   portName.push_back('.');
   unsigned baseLen = portName.size();
   for (unsigned i = 0, e = op.getNumResults(); i < e; ++i) {
-    portName.append(legalize(op.getPortName(i)));
+    portName.append(legalize(op.getPortNameAttr(i)));
     addValueName(op.getResult(i), portName);
     portName.resize(baseLen);
   }

lib/Dialect/FIRRTL/FIRRTLAnnotationHelper.cpp

@@ -29,7 +29,7 @@ using llvm::StringRef;
 static LogicalResult updateExpandedPort(StringRef field, AnnoTarget &ref) {
   if (auto mem = dyn_cast<MemOp>(ref.getOp()))
     for (size_t p = 0, pe = mem.getPortNames().size(); p < pe; ++p)
-      if (mem.getPortNameStr(p) == field) {
+      if (mem.getPortName(p) == field) {
         ref = PortAnnoTarget(mem, p);
         return success();
       }

lib/Dialect/FIRRTL/FIRRTLOps.cpp

@@ -2736,7 +2736,7 @@ InstanceOp InstanceOp::cloneWithInsertedPorts(
     }
 
     newPortDirections.push_back(getPortDirection(i));
-    newPortNames.push_back(getPortName(i));
+    newPortNames.push_back(getPortNameAttr(i));
     newPortTypes.push_back(getType(i));
     newPortAnnos.push_back(getPortAnnotation(i));
     newDomainInfo.push_back(getDomainInfo()[i]);
@@ -2887,7 +2887,7 @@ void InstanceOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
     base = "inst";
 
   for (size_t i = 0, e = (*this)->getNumResults(); i != e; ++i) {
-    setNameFn(getResult(i), (base + "_" + getPortNameStr(i)).str());
+    setNameFn(getResult(i), (base + "_" + getPortName(i)).str());
   }
 }
 
@@ -3219,7 +3219,7 @@ InstanceChoiceOp InstanceChoiceOp::cloneWithInsertedPorts(
     }
 
     newPortDirections.push_back(getPortDirection(i));
-    newPortNames.push_back(getPortName(i));
+    newPortNames.push_back(getPortNameAttr(i));
     newPortTypes.push_back(getType(i));
     newPortAnnos.push_back(getPortAnnotations()[i]);
     newDomainInfo.push_back(getDomainInfo()[i]);
@@ -3343,7 +3343,7 @@ LogicalResult MemOp::verify() {
   FIRRTLType oldDataType;
 
   for (size_t i = 0, e = getNumResults(); i != e; ++i) {
-    auto portName = getPortName(i);
+    auto portName = getPortNameAttr(i);
 
     // Get a bundle type representing this port, stripping an outer
     // flip if it exists.  If this is not a bundle<> or
@@ -3453,10 +3453,10 @@ LogicalResult MemOp::verify() {
     // Error if the type of the current port was not the same as the
     // last port, but skip checking the first port.
     if (oldDataType && oldDataType != dataType) {
-      emitOpError() << "port " << getPortName(i)
-                    << " has a different type than port " << getPortName(i - 1)
-                    << " (expected " << oldDataType << ", but got " << dataType
-                    << ")";
+      emitOpError() << "port " << getPortNameAttr(i)
+                    << " has a different type than port "
+                    << getPortNameAttr(i - 1) << " (expected " << oldDataType
+                    << ", but got " << dataType << ")";
       return failure();
     }
 
@@ -3539,7 +3539,7 @@ SmallVector<MemOp::NamedPort> MemOp::getPorts() {
   for (size_t i = 0, e = getNumResults(); i != e; ++i) {
     // Each port is a bundle.
     auto portType = type_cast<FIRRTLType>(getResult(i).getType());
-    result.push_back({getPortName(i), getMemPortKindFromType(portType)});
+    result.push_back({getPortNameAttr(i), getMemPortKindFromType(portType)});
   }
   return result;
 }
@@ -3596,7 +3596,7 @@ FIRRTLBaseType MemOp::getDataType() {
       .getElementType(dataFieldName);
 }
 
-StringAttr MemOp::getPortName(size_t resultNo) {
+StringAttr MemOp::getPortNameAttr(size_t resultNo) {
   return cast<StringAttr>(getPortNames()[resultNo]);
 }
 
@@ -3717,7 +3717,7 @@ void MemOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
     base = "mem";
 
   for (size_t i = 0, e = (*this)->getNumResults(); i != e; ++i) {
-    setNameFn(getResult(i), (base + "_" + getPortNameStr(i)).str());
+    setNameFn(getResult(i), (base + "_" + getPortName(i)).str());
   }
 }
 
@@ -4278,6 +4278,14 @@ LogicalResult PropAssignOp::verify() {
   return success();
 }
 
+template <typename T>
+static FlatSymbolRefAttr getDomainTypeNameOfResult(T op, size_t i) {
+  auto info = op.getDomainInfo();
+  if (info.empty())
+    return {};
+  return dyn_cast<FlatSymbolRefAttr>(info[i]);
+}
+
 static FlatSymbolRefAttr getDomainTypeName(Value value) {
   if (!isa<DomainType>(value.getType()))
     return {};
@@ -4297,13 +4305,10 @@ static FlatSymbolRefAttr getDomainTypeName(Value value) {
 
   if (auto result = dyn_cast<OpResult>(value)) {
     auto *op = result.getDefiningOp();
-    if (auto instance = dyn_cast<InstanceOp>(op)) {
-      auto info = instance.getDomainInfo();
-      if (info.empty())
-        return {};
-      auto attr = info[result.getResultNumber()];
-      return dyn_cast<FlatSymbolRefAttr>(attr);
-    }
+    if (auto instance = dyn_cast<InstanceOp>(op))
+      return getDomainTypeNameOfResult(instance, result.getResultNumber());
+    if (auto instance = dyn_cast<InstanceChoiceOp>(op))
+      return getDomainTypeNameOfResult(instance, result.getResultNumber());
     return {};
   }
 

lib/Dialect/FIRRTL/FIRRTLReductions.cpp

@@ -414,7 +414,7 @@ struct InstanceStubber : public OpReduction<firrtl::InstanceOp> {
     for (unsigned i = 0, e = instOp.getNumResults(); i != e; ++i) {
       auto result = instOp.getResult(i);
       auto name = builder.getStringAttr(Twine(instOp.getName()) + "_" +
-                                        instOp.getPortNameStr(i));
+                                        instOp.getPortName(i));
       auto wire =
           firrtl::WireOp::create(builder, result.getType(), name,
                                  firrtl::NameKindEnum::DroppableName,
@@ -463,7 +463,7 @@ struct MemoryStubber : public OpReduction<firrtl::MemOp> {
     for (unsigned i = 0, e = memOp.getNumResults(); i != e; ++i) {
       auto result = memOp.getResult(i);
       auto name = builder.getStringAttr(Twine(memOp.getName()) + "_" +
-                                        memOp.getPortNameStr(i));
+                                        memOp.getPortName(i));
       auto wire =
           firrtl::WireOp::create(builder, result.getType(), name,
                                  firrtl::NameKindEnum::DroppableName,
@@ -1190,7 +1190,7 @@ struct EagerInliner : public OpReduction<InstanceOp> {
     for (unsigned i = 0, e = instOp.getNumResults(); i != e; ++i) {
       auto result = instOp.getResult(i);
       auto name = rewriter.getStringAttr(Twine(instOp.getName()) + "_" +
-                                         instOp.getPortNameStr(i));
+                                         instOp.getPortName(i));
       auto wire = WireOp::create(rewriter, instOp.getLoc(), result.getType(),
                                  name, NameKindEnum::DroppableName,
                                  instOp.getPortAnnotation(i), StringAttr{})

lib/Dialect/FIRRTL/FIRRTLUtils.cpp

@@ -571,8 +571,7 @@ static void getDeclName(Value value, SmallString<64> &string, bool nameSafe) {
         .Case<InstanceOp, MemOp>([&](auto op) {
           string += op.getName();
           string += nameSafe ? "_" : ".";
-          string += op.getPortName(cast<OpResult>(value).getResultNumber())
-                        .getValue();
+          string += op.getPortName(cast<OpResult>(value).getResultNumber());
           value = nullptr;
         })
         .Case<FNamableOp>([&](auto op) {

lib/Dialect/FIRRTL/Transforms/CMakeLists.txt

@@ -17,6 +17,7 @@ add_circt_dialect_library(CIRCTFIRRTLTransforms
   GrandCentral.cpp
   IMConstProp.cpp
   IMDeadCodeElim.cpp
+  InferDomains.cpp
   InferReadWrite.cpp
   InferResets.cpp
   InferWidths.cpp

lib/Dialect/FIRRTL/Transforms/Dedup.cpp

@@ -1596,7 +1596,7 @@ fixupSymbolSensitiveOp(Operation *op, InstanceGraph &instanceGraph,
         continue;
       LLVM_DEBUG(llvm::dbgs()
                  << "- Updating instance port \"" << instOp.getInstanceName()
-                 << "." << instOp.getPortName(index).getValue() << "\" from "
+                 << "." << instOp.getPortName(index) << "\" from "
                  << oldType << " to " << newType << "\n");
 
       // If the type changed we transform it back to the old type with an

lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp

@@ -563,7 +563,7 @@ void ExtractInstancesPass::extractInstances() {
     for (unsigned portIdx = 0; portIdx < numInstPorts; ++portIdx) {
       // Assemble the new port name as "<prefix>_<name>", where the prefix is
       // provided by the extraction annotation.
-      auto name = inst.getPortNameStr(portIdx);
+      auto name = inst.getPortName(portIdx);
       auto nameAttr = StringAttr::get(
           &getContext(),
           prefix.empty() ? Twine(name) : Twine(prefix) + "_" + name);
@@ -913,7 +913,7 @@ void ExtractInstancesPass::groupInstances() {
       StringRef prefix(instPrefixNamesPair[inst].first);
       unsigned portNum = inst.getNumResults();
       for (unsigned portIdx = 0; portIdx < portNum; ++portIdx) {
-        auto name = inst.getPortNameStr(portIdx);
+        auto name = inst.getPortName(portIdx);
         auto nameAttr = builder.getStringAttr(
             prefix.empty() ? Twine(name) : Twine(prefix) + "_" + name);
         PortInfo port{nameAttr,

lib/Dialect/FIRRTL/Transforms/FlattenMemory.cpp

@@ -138,7 +138,7 @@ struct FlattenMemoryPass
         auto result = memOp.getResult(index);
         auto wire = WireOp::create(builder, result.getType(),
                                    (memOp.getName() + "_" +
-                                    memOp.getPortName(index).getValue())
+                                    memOp.getPortName(index))
                                        .str())
                         .getResult();
         result.replaceAllUsesWith(wire);