[firtool] fix: check uninferred resets after removing unused modules

The Chisel's Definition API generates modules that are not instantiated,
whose reset cannot be inferred properly. Check the uninferred resets
after removing unreferenced modules to resolve it.

Signed-off-by: unlsycn <[email protected]>

include/circt/Dialect/FIRRTL/Passes.h

@@ -214,6 +214,8 @@ std::unique_ptr<mlir::Pass> createLowerDPIPass();
 std::unique_ptr<mlir::Pass>
 createAssignOutputDirsPass(mlir::StringRef outputDir = "");
 
+std::unique_ptr<mlir::Pass> createCheckUninferredResetsPass();
+
 /// Generate the code for registering passes.
 #define GEN_PASS_REGISTRATION
 #include "circt/Dialect/FIRRTL/Passes.h.inc"

include/circt/Dialect/FIRRTL/Passes.td

@@ -970,4 +970,16 @@ def ProbesToSignals : Pass<"firrtl-probes-to-signals", "firrtl::CircuitOp"> {
   let constructor = "circt::firrtl::createProbesToSignalsPass()";
 }
 
+def CheckUninferredResets : Pass<"firrtl-check-uninferred-resets", "firrtl::CircuitOp"> {
+  let summary = "Verify no uninferred resets";
+  let description = [{
+    This pass checks to see if there are abstract type resets which have not
+    been inferred correctly.
+
+    Run after Inliner since the reset type of an unreferenced module (which
+    doesn't have a parent module) cannot be inferred.
+  }];
+  let constructor = "circt::firrtl::createCheckUninferredResetsPass()";
+}
+
 #endif // CIRCT_DIALECT_FIRRTL_PASSES_TD

lib/Dialect/FIRRTL/Transforms/CMakeLists.txt

@@ -3,6 +3,7 @@ add_circt_dialect_library(CIRCTFIRRTLTransforms
   AddSeqMemPorts.cpp
   BlackBoxReader.cpp
   CheckCombLoops.cpp
+  CheckUninferredResets.cpp
   CreateCompanionAssume.cpp
   CreateSiFiveMetadata.cpp
   Dedup.cpp

lib/Dialect/FIRRTL/Transforms/CheckUninferredResets.cpp

@@ -0,0 +1,62 @@
+//===- CheckUninferredResets.cpp - Verify no uninferred resets ------===//
+//
+// 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 pass checks to see if there are abstract type resets which have not
+// been inferred correctly.
+//
+//===----------------------------------------------------------------------===//
+
+#include "circt/Dialect/FIRRTL/FIRRTLOpInterfaces.h"
+#include "circt/Dialect/FIRRTL/FIRRTLUtils.h"
+#include "circt/Dialect/FIRRTL/Passes.h"
+#include "mlir/Pass/Pass.h"
+
+namespace circt {
+namespace firrtl {
+#define GEN_PASS_DEF_CHECKUNINFERREDRESETS
+#include "circt/Dialect/FIRRTL/Passes.h.inc"
+} // namespace firrtl
+} // namespace circt
+
+using namespace mlir;
+using namespace circt;
+using namespace firrtl;
+
+namespace {
+struct CheckUninferredResetsPass
+    : public circt::firrtl::impl::CheckUninferredResetsBase<
+          CheckUninferredResetsPass> {
+  void runOnOperation() override;
+
+  LogicalResult verifyNoAbstractReset();
+};
+} // namespace
+
+void CheckUninferredResetsPass::runOnOperation() {
+  bool hasAbstractResetPorts = false;
+  for (FModuleLike module :
+       getOperation().getBodyBlock()->getOps<FModuleLike>()) {
+    for (PortInfo port : module.getPorts()) {
+      if (getBaseOfType<ResetType>(port.type)) {
+        auto diag = emitError(port.loc)
+                    << "a port \"" << port.getName()
+                    << "\" with abstract reset type was unable to be "
+                       "inferred by InferResets (is this a top-level port?)";
+        diag.attachNote(module->getLoc())
+            << "the module with this uninferred reset port was defined here";
+        hasAbstractResetPorts = true;
+      }
+    }
+  }
+  if (hasAbstractResetPorts)
+    return signalPassFailure();
+}
+
+std::unique_ptr<mlir::Pass> circt::firrtl::createCheckUninferredResetsPass() {
+  return std::make_unique<CheckUninferredResetsPass>();
+}

lib/Dialect/FIRRTL/Transforms/InferResets.cpp

@@ -461,8 +461,6 @@ struct InferResetsPass
   LogicalResult implementFullReset(FModuleOp module, ResetDomain &domain);
   void implementFullReset(Operation *op, FModuleOp module, Value actualReset);
 
-  LogicalResult verifyNoAbstractReset();
-
   //===--------------------------------------------------------------------===//
   // Utilities
 
@@ -546,10 +544,6 @@ void InferResetsPass::runOnOperationInner() {
   // Implement the full resets.
   if (failed(implementFullReset()))
     return signalPassFailure();
-
-  // Require that no Abstract Resets exist on ports in the design.
-  if (failed(verifyNoAbstractReset()))
-    return signalPassFailure();
 }
 
 std::unique_ptr<mlir::Pass> circt::firrtl::createInferResetsPass() {
@@ -1909,25 +1903,3 @@ void InferResetsPass::implementFullReset(Operation *op, FModuleOp module,
     regOp.getResetValueMutable().assign(zero);
   }
 }
-
-LogicalResult InferResetsPass::verifyNoAbstractReset() {
-  bool hasAbstractResetPorts = false;
-  for (FModuleLike module :
-       getOperation().getBodyBlock()->getOps<FModuleLike>()) {
-    for (PortInfo port : module.getPorts()) {
-      if (getBaseOfType<ResetType>(port.type)) {
-        auto diag = emitError(port.loc)
-                    << "a port \"" << port.getName()
-                    << "\" with abstract reset type was unable to be "
-                       "inferred by InferResets (is this a top-level port?)";
-        diag.attachNote(module->getLoc())
-            << "the module with this uninferred reset port was defined here";
-        hasAbstractResetPorts = true;
-      }
-    }
-  }
-
-  if (hasAbstractResetPorts)
-    return failure();
-  return success();
-}

lib/Firtool/Firtool.cpp

@@ -142,6 +142,9 @@ LogicalResult firtool::populateCHIRRTLToLowFIRRTL(mlir::PassManager &pm,
 
   pm.nest<firrtl::CircuitOp>().addPass(firrtl::createInlinerPass());
 
+  pm.nest<firrtl::CircuitOp>().addPass(
+      firrtl::createCheckUninferredResetsPass());
+
   // Preset the random initialization parameters for each module. The current
   // implementation assumes it can run at a time where every register is
   // currently in the final module it will be emitted in, all registers have

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

@@ -1,4 +1,4 @@
-// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets))' --verify-diagnostics --split-input-file %s
+// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets,firrtl-check-uninferred-resets))' --verify-diagnostics --split-input-file %s
 
 // Tests extracted from:
 // - github.com/chipsalliance/firrtl:

test/firtool/legal-uninferred-resets.fir

@@ -0,0 +1,31 @@
+; RUN: firtool %s --verify-diagnostics --split-input-file
+; The Chisel's Definition API generates modules that are not instantiated,
+; whose reset cannot be inferred properly. These modules should be removed
+; before checking abstract type resets.
+
+FIRRTL version 3.3.0
+circuit Foo:
+  module Adder :
+    input clock : Clock
+    input reset : Reset
+    input in : UInt<10>
+    output out : UInt<10>
+
+    node _out_T = add(in, UInt<1>(0h1))
+    node _out_T_1 = tail(_out_T, 1)
+    connect out, _out_T_1
+
+  ; expected-warning @below {{module `Foo` is empty}}
+  module Foo :
+    input clock : Clock
+    input reset : UInt<1>
+
+;// -----
+
+FIRRTL version 3.3.0
+circuit Bar:
+    ; expected-note @below {{the module with this uninferred reset port was defined here}}
+  module Bar :
+    input clock : Clock
+    ; expected-error @below {{a port "reset" with abstract reset type was unable to be inferred by InferResets}}
+    input reset : Reset