[FIRRTL] Grand Central Testharness Fixes (#2864)

Fix Grand Central to properly handle views/interfaces that are
instantiated in the test harness and not in the DUT.  Specifically,
interfaces and companions are not extracted via binds if they are only
instantiated in the testharness (and a DUT is known via a
MarkDUTAnnotation).

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp

@@ -576,6 +576,15 @@ private:
   /// before the pass finishes.
   DenseSet<StringAttr> deadNLAs;
 
+  /// The design-under-test (DUT) as determined by the presence of a
+  /// "sifive.enterprise.firrtl.MarkDUTAnnotation".  This will be null if no DUT
+  /// was found.
+  FModuleLike dut;
+
+  /// An optional directory for testbench-related files.  This is null if no
+  /// "TestBenchDirAnnotation" is found.
+  StringAttr testbenchDir;
+
   /// Return a string containing the name of an interface.  Apply correct
   /// prefixing from the interfacePrefix and module-level prefix parameter.
   std::string getInterfaceName(StringAttr prefix,
@@ -1013,7 +1022,16 @@ GrandCentralPass::traverseBundle(AugmentedBundleTypeAttr bundle, IntegerAttr id,
   auto loc = getOperation().getLoc();
   auto iFaceName = getNamespace().newName(getInterfaceName(prefix, bundle));
   iface = builder.create<sv::InterfaceOp>(loc, iFaceName);
-  if (maybeExtractInfo)
+  if (dut &&
+      !instancePaths->instanceGraph.isAncestor(companionIDMap[id].companion,
+                                               cast<hw::HWModuleLike>(*dut)) &&
+      testbenchDir)
+    iface->setAttr("output_file",
+                   hw::OutputFileAttr::getFromDirectoryAndFilename(
+                       &getContext(), testbenchDir.getValue(),
+                       iFaceName + ".sv",
+                       /*excludFromFileList=*/true));
+  else if (maybeExtractInfo)
     iface->setAttr("output_file",
                    hw::OutputFileAttr::getFromDirectoryAndFilename(
                        &getContext(), getOutputDirectory().getValue(),
@@ -1192,9 +1210,34 @@ void GrandCentralPass::runOnOperation() {
       interfacePrefix = prefix.getValue();
       return true;
     }
+    if (anno.isClass(testbenchDirAnnoClass)) {
+      testbenchDir = anno.getMember<StringAttr>("dirname");
+      return false;
+    }
     return false;
   });
 
+  // Find the DUT if it exists.  This needs to be known before the circuit is
+  // walked.
+  for (auto mod : circuitOp.getOps<FModuleLike>()) {
+    if (!AnnotationSet(mod).hasAnnotation(dutAnnoClass))
+      continue;
+
+    // TODO: This check is duplicated multiple places, e.g., in
+    // WireDFT.  This should be factored out as part of the annotation
+    // lowering pass.
+    if (dut) {
+      auto diag = emitError(mod.getLoc())
+                  << "is marked with a '" << dutAnnoClass << "', but '"
+                  << dut.moduleName()
+                  << "' also had such an annotation (this should "
+                     "be impossible!)";
+      diag.attachNote(dut.getLoc()) << "the first DUT was found here";
+      removalError = true;
+    }
+    dut = mod;
+  }
+
   if (removalError)
     return signalPassFailure();
 
@@ -1212,6 +1255,11 @@ void GrandCentralPass::runOnOperation() {
                    << "\n";
     else
       llvm::dbgs() << "  <none>\n";
+    llvm::dbgs() << "DUT: ";
+    if (dut)
+      llvm::dbgs() << dut.moduleName() << "\n";
+    else
+      llvm::dbgs() << "<none>\n";
     llvm::dbgs()
         << "Prefix Info (from PrefixInterfacesAnnotation):\n"
         << "  prefix: " << interfacePrefix << "\n"
@@ -1399,7 +1447,9 @@ void GrandCentralPass::runOnOperation() {
             //   3. Instatiate the mapping module in the companion.
             //   4. Check that the companion is instantated exactly once.
             //   5. Set attributes on that lone instance so it will become a
-            //      bind if extraction information was provided.
+            //      bind if extraction information was provided.  If a DUT is
+            //      known, then anything in the test harness will not be
+            //      extracted.
             if (tpe.getValue() == "companion") {
               builder.setInsertionPointToEnd(circuitOp.getBody());
 
@@ -1436,6 +1486,12 @@ void GrandCentralPass::runOnOperation() {
               if (!maybeExtractInfo)
                 return true;
 
+              // If the companion is instantiated above the DUT, then don't
+              // extract it.
+              if (dut && !instancePaths->instanceGraph.isAncestor(
+                             op, cast<hw::HWModuleLike>(*dut)))
+                return true;
+
               instance.getValue()->setAttr("lowerToBind", trueAttr);
               instance.getValue()->setAttr(
                   "output_file",
@@ -1673,6 +1729,13 @@ void GrandCentralPass::runOnOperation() {
     if (!maybeExtractInfo)
       continue;
 
+    // If the interface is associated with a companion that is instantiated
+    // above the DUT (e.g.., in the test harness), then don't extract it.
+    if (dut && !instancePaths->instanceGraph.isAncestor(
+                   companionIDMap[bundle.getID()].companion,
+                   cast<hw::HWModuleLike>(*dut)))
+      continue;
+
     instance->setAttr("doNotPrint", trueAttr);
     builder.setInsertionPointToStart(
         instance->getParentOfType<CircuitOp>().getBody());
@@ -1695,6 +1758,7 @@ void GrandCentralPass::runOnOperation() {
     llvm::yaml::Output yout(stream);
     yamlize(yout, interfaceVec, true, yamlContext);
 
+    builder.setInsertionPointToStart(circuitOp.getBody());
     builder.create<sv::VerbatimOp>(builder.getUnknownLoc(), yamlString)
         ->setAttr("output_file",
                   hw::OutputFileAttr::getFromFilename(

test/Dialect/FIRRTL/grand-central.mlir

@@ -1081,6 +1081,69 @@ firrtl.circuit "BlackBoxDirectoryBehavior" attributes {
 
 // -----
 
+firrtl.circuit "InterfaceInTestHarness" attributes {
+  annotations = [
+    {class = "sifive.enterprise.grandcentral.AugmentedBundleType",
+     defName = "Foo",
+     elements = [
+       {class = "sifive.enterprise.grandcentral.AugmentedGroundType",
+        description = "description of foo",
+        name = "foo",
+        id = 1 : i64}],
+     id = 0 : i64,
+     name = "View"},
+    {class = "sifive.enterprise.grandcentral.ExtractGrandCentralAnnotation",
+     directory = "gct-dir",
+     filename = "gct-dir/bindings.sv"},
+    {class = "sifive.enterprise.firrtl.TestBenchDirAnnotation",
+     dirname = "testbenchDir"}]} {
+  firrtl.module @View_companion() attributes {
+    annotations = [
+      {class = "sifive.enterprise.grandcentral.ViewAnnotation",
+       defName = "Foo",
+       id = 0 : i64,
+       name = "View",
+       type = "companion"}]} {}
+  firrtl.module @DUT() attributes {
+    annotations = [
+      {class = "sifive.enterprise.firrtl.MarkDUTAnnotation"}
+    ]} {
+  }
+  firrtl.module @InterfaceInTestHarness() attributes {
+    annotations = [
+      {class = "sifive.enterprise.grandcentral.ViewAnnotation",
+       id = 0 : i64,
+       name = "view",
+       type = "parent"}]} {
+    firrtl.instance dut @DUT()
+    %a = firrtl.wire {annotations = [
+      {class = "sifive.enterprise.grandcentral.AugmentedGroundType",
+       id = 1 : i64},
+      {class = "firrtl.transforms.DontTouchAnnotation"}]} : !firrtl.uint<2>
+    firrtl.instance View_companion @View_companion()
+  }
+}
+
+// Check that an interface, instantiated inside the test harness (technically,
+// in a module which is not a child of the marked Design Under Test), will not
+// be extracted via a bind.  Also, check that interfaces only inside the test
+// harness will be written to the test harness directory.
+//
+// CHECK-LABEL: "InterfaceInTestHarness"
+// CHECK:       firrtl.module @InterfaceInTestHarness
+// CHECK:         firrtl.instance View_companion
+// CHECK-NOT:       output_file
+// CHECK-NOT:       lowerToBind
+// CHECK:         sv.interface.instance
+// CHECK-NOT:       output_file
+// CHECK-NOT:       lowerToBind
+// CHECK-SAME:      !sv.interface
+// CHECK-NEXT:  }
+// CHECK:       sv.interface
+// CHECK-SAME:    output_file = #hw.output_file<"testbenchDir/Foo.sv", excludeFromFileList>
+
+// -----
+
 firrtl.circuit "YAMLOutputEmptyInterface" attributes {
   annotations = [
     {class = "sifive.enterprise.grandcentral.AugmentedBundleType",