From 08a931e3ec03ced52f2a3194f8b4c7fbe294adae Mon Sep 17 00:00:00 2001
From: Prithayan Barua <prithayan@gmail.com>
Date: Mon, 25 Oct 2021 14:18:32 -0400
Subject: [PATCH] [FIRRTL] Fix check for multiple instances in Grand Central
 (#2034)

Fix the assumption in `GrandCentral` that only `InstanceOp` is a user of
 `firrtl.module` symbol, because `NonLocalAnchor` can also be a user.
---
 .../FIRRTL/Transforms/GrandCentral.cpp        | 15 ++++---
 test/Dialect/FIRRTL/grand-central-errors.mlir | 39 +++++++++++++++++++
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp
index 16ef3003edab..3f7d089f6a32 100644
--- a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp
+++ b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp
@@ -627,19 +627,24 @@ void GrandCentralPass::runOnOperation() {
   // the module is not instantiated or is multiply instantiated.
   auto exactlyOneInstance = [&](FModuleOp op,
                                 StringRef msg) -> Optional<InstanceOp> {
-    auto instances = getSymbolTable().getSymbolUses(op, circuitOp);
-    if (!instances.hasValue()) {
+    auto uses = getSymbolTable().getSymbolUses(op, circuitOp);
+
+    auto instances = llvm::make_filter_range(uses.getValue(), [&](auto u) {
+      return (isa<InstanceOp>(u.getUser()));
+    });
+
+    if (instances.empty()) {
       op->emitOpError() << "is marked as a GrandCentral '" << msg
                         << "', but is never instantiated";
       return None;
     }
 
-    if (llvm::hasSingleElement(instances.getValue()))
-      return cast<InstanceOp>((*(instances.getValue().begin())).getUser());
+    if (llvm::hasSingleElement(instances))
+      return cast<InstanceOp>((*(instances.begin())).getUser());
 
     auto diag = op->emitOpError() << "is marked as a GrandCentral '" << msg
                                   << "', but it is instantiated more than once";
-    for (auto instance : instances.getValue())
+    for (auto instance : instances)
       diag.attachNote(instance.getUser()->getLoc())
           << "parent is instantiated here";
     return None;
diff --git a/test/Dialect/FIRRTL/grand-central-errors.mlir b/test/Dialect/FIRRTL/grand-central-errors.mlir
index 9a7fe98daf23..917cb60b63c2 100644
--- a/test/Dialect/FIRRTL/grand-central-errors.mlir
+++ b/test/Dialect/FIRRTL/grand-central-errors.mlir
@@ -188,3 +188,42 @@ firrtl.circuit "Foo" attributes {
     firrtl.instance dut @DUT()
   }
 }
+
+
+// ----- 
+// expected-error @+1 {{'firrtl.circuit' op contains a 'companion' with id '0', but does not contain a GrandCentral 'parent' with the same id}}
+firrtl.circuit "multiInstance2" attributes {
+  annotations = [
+    {class = "sifive.enterprise.grandcentral.AugmentedBundleType",
+     defName = "Foo",
+     id = 0 : i64,
+     name = "View"},
+    {class = "sifive.enterprise.grandcentral.ExtractGrandCentralAnnotation",
+     directory = "gct-dir",
+     filename = "gct-dir/bindings.sv"}]} {
+  firrtl.module @View_companion() attributes {
+    annotations = [
+      {class = "sifive.enterprise.grandcentral.ViewAnnotation",
+       defName = "Foo",
+       id = 0 : i64,
+       name = "View",
+       type = "companion"}]} {}
+  // expected-error @+1 {{'firrtl.module' op is marked as a GrandCentral 'parent', but it is instantiated more than once}}
+  firrtl.module @DUTE() attributes {
+    annotations = [
+      {class = "sifive.enterprise.grandcentral.ViewAnnotation",
+       id = 0 : i64,
+       name = "view",
+       type = "parent"}
+    ]} {
+    %a = firrtl.wire : !firrtl.uint<2>
+    %b = firrtl.wire : !firrtl.uint<4>
+    firrtl.instance View_companion @View_companion()
+  }
+  firrtl.module @multiInstance2() {
+    firrtl.instance dut @DUTE() // expected-note {{parent is instantiated here}}
+    firrtl.instance dut1 @DUTE() // expected-note {{parent is instantiated here}}
+  }
+  firrtl.nla @nla1 [@multiInstance1] ["dut"]
+  firrtl.nla @nla2 [@multiInstance1] ["dut1"]
+}