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

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 : InterfacePass<"firrtl-check-uninferred-resets", "firrtl::FModuleLike"> {
+  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,55 @@
+//===- 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;
+};
+} // namespace
+
+void CheckUninferredResetsPass::runOnOperation() {
+  auto module = getOperation();
+  for (auto 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";
+      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>().addNestedPass<firrtl::FModuleOp>(
+      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,any(firrtl-check-uninferred-resets)))' --verify-diagnostics --split-input-file %s
 
 // Tests extracted from:
 // - github.com/chipsalliance/firrtl:

test/Dialect/FIRRTL/legal-uninferred-resets.mlir

@@ -0,0 +1,21 @@
+// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets,any(firrtl-check-uninferred-resets)))' --verify-diagnostics %s
+// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets,firrtl-inliner,any(firrtl-check-uninferred-resets)))' %s | FileCheck %s
+
+firrtl.circuit "LegalUninferredReset" {
+  // The following two diagnostics will only appear if firrtl-inliner is not enabled
+  // expected-note @+2 {{the module with this uninferred reset port was defined here}}
+  // expected-error @+1 {{a port "reset" with abstract reset type was unable to be inferred by InferResets}}
+  firrtl.module private @Adder(in %clock: !firrtl.clock, in %reset: !firrtl.reset, in %in: !firrtl.uint<10>, out %out: !firrtl.uint<10>) {
+    %c1_ui1 = firrtl.constant 1 : !firrtl.const.uint<1>
+    %0 = firrtl.add %in, %c1_ui1 : (!firrtl.uint<10>, !firrtl.const.uint<1>) -> !firrtl.uint<11>
+    %_out_T = firrtl.node interesting_name %0 : !firrtl.uint<11>
+    %1 = firrtl.tail %_out_T, 1 : (!firrtl.uint<11>) -> !firrtl.uint<10>
+    %_out_T_1 = firrtl.node interesting_name %1 : !firrtl.uint<10>
+    firrtl.matchingconnect %out, %_out_T_1 : !firrtl.uint<10>
+  }
+  firrtl.module @LegalUninferredReset(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>) {
+  }
+}
+
+// CHECK-NOT: firrtl.circuit "Adder"
+// CHECK: firrtl.circuit "LegalUninferredReset"

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