[ETC] Don't relocate instances that were only in extracted test code. (…

…#5064) (#5065)

These don't actually belong in the testbench directory; they are still
part of the design. So for now, simply leave them in the directory
they were initially declared.

docs/Dialects/FIRRTL/FIRRTLAnnotations.md

@@ -1463,11 +1463,6 @@ modules. This attribute has type `OutputFileAttr`.
 Used by SVExtractTestCode.  Specifies the output directory for extracted
 modules. This attribute has type `OutputFileAttr`.
 
-### firrtl.extract.testbench
-
-Used by SVExtractTestCode.  Specifies the output directory for extracted
-testbench only modules. This attribute has type `OutputFileAttr`.
-
 ### firrtl.extract.assert.bindfile
 
 Used by SVExtractTestCode.  Specifies the output file for extracted

lib/Conversion/FIRRTLToHW/LowerToHW.cpp

@@ -505,15 +505,6 @@ void FIRRTLModuleLowering::runOnOperation() {
   circuitAnno.removeAnnotationsWithClass(
       extractAssertAnnoClass, extractAssumeAnnoClass, extractCoverageAnnoClass);
 
-  // Pass along the testbench directory for ExtractTestCode to use later.
-  if (auto tbAnno = circuitAnno.getAnnotation(testBenchDirAnnoClass)) {
-    auto dirName = tbAnno.getMember<StringAttr>("dirname");
-    auto testBenchDir = hw::OutputFileAttr::getAsDirectory(
-        &getContext(), dirName.getValue(), /*excludeFromFileList=*/true,
-        /*includeReplicatedOps=*/true);
-    getOperation()->setAttr("firrtl.extract.testbench", testBenchDir);
-  }
-
   state.processRemainingAnnotations(circuit, circuitAnno);
   // Iterate through each operation in the circuit body, transforming any
   // FModule's we come across. If any module fails to lower, return early.

lib/Dialect/SV/Transforms/SVExtractTestCode.cpp

@@ -657,35 +657,6 @@ struct SVExtractTestCodeImplPass
     return true;
   }
 
-  // Move any modules that had all instances extracted to the testbench path.
-  void maybeMoveExtractedModules(hw::InstanceGraph &instanceGraph,
-                                 Attribute testBenchDir) {
-    // Ensure we have a valid test code path.
-    if (!testBenchDir)
-      return;
-
-    // Check each module that had instances extracted.
-    for (auto &pair : extractedInstances) {
-      hw::InstanceGraphNode *node = instanceGraph.lookup(pair.first);
-      assert(!node->noUses() && "expected module whose instances were "
-                                "extracted to be instantiated at least once");
-
-      // See if all instances were extracted.
-      bool allInstancesExtracted = true;
-      for (hw::InstanceRecord *use : node->uses()) {
-        allInstancesExtracted &= extractedInstances[pair.first].contains(
-            use->getInstance().getOperation());
-      }
-
-      if (!allInstancesExtracted)
-        continue;
-
-      // If so, move the module to the test code path.
-      hw::HWModuleLike mod = node->getModule();
-      mod->setAttr("output_file", testBenchDir);
-    }
-  }
-
   // Map from module name to set of extracted instances for that module.
   DenseMap<StringAttr, SmallPtrSet<Operation *, 32>> extractedInstances;
 
@@ -706,8 +677,6 @@ void SVExtractTestCodeImplPass::runOnOperation() {
       top->getAttrOfType<hw::OutputFileAttr>("firrtl.extract.assume");
   auto coverDir =
       top->getAttrOfType<hw::OutputFileAttr>("firrtl.extract.cover");
-  auto testBenchDir =
-      top->getAttrOfType<hw::OutputFileAttr>("firrtl.extract.testbench");
   auto assertBindFile =
       top->getAttrOfType<hw::OutputFileAttr>("firrtl.extract.assert.bindfile");
   auto assumeBindFile =
@@ -807,10 +776,6 @@ void SVExtractTestCodeImplPass::runOnOperation() {
     }
   }
 
-  // After all instances are processed, move any modules that had all instances
-  // extracted to the testbench path.
-  maybeMoveExtractedModules(*instanceGraph, testBenchDir);
-
   // We have to wait until all the instances are processed to clean up the
   // annotations.
   for (auto &op : topLevelModule->getOperations())
@@ -819,7 +784,6 @@ void SVExtractTestCodeImplPass::runOnOperation() {
       op.removeAttr("firrtl.extract.cover.extra");
       op.removeAttr("firrtl.extract.assume.extra");
     }
-  top->removeAttr("firrtl.extract.testbench");
 
   markAnalysesPreserved<circt::hw::InstanceGraph>();
 }

test/Dialect/SV/hw-extract-test-code.mlir

@@ -227,14 +227,6 @@ module {
 // -----
 // Check instance extraction
 
-// All instances of Baz are extracted, so it should be output to the testbench.
-// CHECK-LABEL: @Baz
-// CHECK-SAME: output_file = #hw.output_file<"testbench{{/|\\\\}}", excludeFromFileList, includeReplicatedOps>
-
-// All instances of Bozo are extracted, so it should be output to the testbench.
-// CHECK-LABEL: @Bozo
-// CHECK: #hw.output_file<"testbench
-
 // In AllExtracted, instances foo, bar, and baz should be extracted.
 // CHECK-LABEL: @AllExtracted_cover
 // CHECK: hw.instance "foo"
@@ -294,9 +286,7 @@ module {
 // CHECK: hw.instance "non_testcode_and_instance0"
 // CHECK: hw.instance "non_testcode_and_instance1"
 
-module attributes {
-  firrtl.extract.testbench = #hw.output_file<"testbench/", excludeFromFileList, includeReplicatedOps>
-} {
+module {
   hw.module private @Foo(%a: i1) -> (b: i1) {
     hw.output %a : i1
   }