Review comments

Author: rwy7

include/circt/Dialect/FIRRTL/FIRRTLOpInterfaces.h

@@ -85,28 +85,6 @@ 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'.

lib/Dialect/FIRRTL/Transforms/InferDomains.cpp

@@ -102,7 +102,8 @@ using DomainTypeID = size_t;
 /// type ID, which in this pass is the canonical way to reference the type
 /// of a domain.
 namespace {
-struct CircuitDomainInfo {
+class CircuitDomainInfo {
+public:
   CircuitDomainInfo(CircuitOp circuit) { processCircuit(circuit); }
 
   ArrayRef<DomainOp> getDomains() const { return domainTable; }
@@ -302,6 +303,7 @@ Term *find(Term *x) {
 LogicalResult unify(Term *lhs, Term *rhs);
 
 LogicalResult unify(VariableTerm *x, Term *y) {
+  assert(!x->leader);
   x->leader = y;
   return success();
 }

lib/Firtool/Firtool.cpp

@@ -49,9 +49,9 @@ LogicalResult firtool::populatePreprocessTransforms(mlir::PassManager &pm,
   pm.nest<firrtl::CircuitOp>().nest<firrtl::FModuleOp>().addPass(
       firrtl::createLowerIntrinsics());
 
-  if (auto mode = toInferDomainsPassMode(opt.getDomainMode())) {
+  if (auto mode = toInferDomainsPassMode(opt.getDomainMode()))
     pm.nest<firrtl::CircuitOp>().addPass(firrtl::createInferDomains({*mode}));
-  }
+
   return success();
 }
 

test/Dialect/FIRRTL/infer-domains-errors.mlir

@@ -15,7 +15,7 @@ firrtl.circuit "DomainCrossOnPort" {
 
 // -----
 
-// Illegal domain crossing - connect op.
+// Illegal domain crossing via connect op.
 firrtl.circuit "IllegalDomainCrossing" {
   firrtl.domain @ClockDomain
   firrtl.module @IllegalDomainCrossing(