Read weird alternate path syntax used by GrandCentral for paths (llvm…

…#2037) This will reveal more unhandled non-local annotations.
hunterzju · Oct 26, 2021 · edd8956 · edd8956
1 parent d1354e5
commit edd8956
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 16 deletions.
diff --git a/lib/Dialect/FIRRTL/Import/FIRAnnotations.cpp b/lib/Dialect/FIRRTL/Import/FIRAnnotations.cpp
@@ -618,15 +618,34 @@ static Optional<DictionaryAttr> parseAugmentedType(
     if (!circuitAttr || !moduleAttr || !pathAttr || !componentAttr)
       return llvm::Optional<std::pair<std::string, ArrayAttr>>();
 
-    // TODO: Enable support for non-local annotations.
-    if (!pathAttr.empty()) {
-      auto diag = mlir::emitError(
-          loc,
-          "Annotation '" + clazz + "' with path '" + path +
-              "' encodes an unsupported non-local target via the 'path' key.");
-
-      diag.attachNote() << "The encoded target is: " << refTarget;
-      return llvm::Optional<std::pair<std::string, ArrayAttr>>();
+    // Parse non-local annotations.
+    SmallString<32> strpath;
+    for (auto p : pathAttr) {
+      auto dict = p.dyn_cast_or_null<DictionaryAttr>();
+      if (!dict) {
+        mlir::emitError(loc, "annotation '" + clazz +
+                                 " has invalid type (expected DictionaryAttr)");
+        return {};
+      }
+      auto instHolder =
+          tryGetAs<DictionaryAttr>(dict, dict, "_1", loc, clazz, path);
+      auto modHolder =
+          tryGetAs<DictionaryAttr>(dict, dict, "_2", loc, clazz, path);
+      if (!instHolder || !modHolder) {
+        mlir::emitError(loc, "annotation '" + clazz +
+                                 " has invalid type (expected DictionaryAttr)");
+        return {};
+      }
+      auto inst = tryGetAs<StringAttr>(instHolder, instHolder, "value", loc,
+                                       clazz, path);
+      auto mod =
+          tryGetAs<StringAttr>(modHolder, modHolder, "value", loc, clazz, path);
+      if (!inst || !mod) {
+        mlir::emitError(loc, "annotation '" + clazz +
+                                 " has invalid type (expected DictionaryAttr)");
+        return {};
+      }
+      strpath += "/" + inst.getValue().str() + ":" + mod.getValue().str();
     }
 
     auto refAttr =
@@ -637,15 +656,14 @@ static Optional<DictionaryAttr> parseAugmentedType(
       auto component = componentAttr[i];
       auto dict = component.dyn_cast_or_null<DictionaryAttr>();
       if (!dict) {
-        mlir::emitError(loc,
-                        "Annotation '" + clazz + "' with path '" + cPath +
-                            " has invalid type (expected DictionaryAttr).");
-        return llvm::Optional<std::pair<std::string, ArrayAttr>>();
+        mlir::emitError(loc, "annotation '" + clazz + "' with path '" + cPath +
+                                 " has invalid type (expected DictionaryAttr)");
+        return {};
       }
       auto classAttr =
           tryGetAs<StringAttr>(dict, refTarget, "class", loc, clazz, cPath);
       if (!classAttr)
-        return llvm::Optional<std::pair<std::string, ArrayAttr>>();
+        return {};
 
       auto value = dict.get("value");
 
@@ -673,12 +691,12 @@ static Optional<DictionaryAttr> parseAugmentedType(
                           "for subfield  or IntegerAttr for subindex).")
               .attachNote()
           << "The value received was: " << value << "\n";
-      return llvm::Optional<std::pair<std::string, ArrayAttr>>();
+      return {};
     }
 
     return llvm::Optional<std::pair<std::string, ArrayAttr>>(
         {(Twine("~" + circuitAttr.getValue() + "|" + moduleAttr.getValue() +
-                ">" + refAttr.getValue()))
+                strpath + ">" + refAttr.getValue()))
              .str(),
          ArrayAttr::get(context, componentAttrs)});
   };

diff --git a/test/Dialect/FIRRTL/annotations-errors.fir b/test/Dialect/FIRRTL/annotations-errors.fir
@@ -153,3 +153,72 @@ circuit Foo: %[[{"a":null,"target":"~Foo|Foo/baz:Bar"}]]
     skip
   module Foo:
     inst bar of Bar
+
+; // -----
+; This should work, but for now we are just checking that the annotation parses 
+; into a path, not that the application is correct.  This is pending GC handling 
+; nlas. 
+
+; expected-error @+2 {{unapplied annotations with target '~NLAParse|A_companion' and payload '[{class = "sifive.enterprise.grandcentral.ViewAnnotation", id = 0 : i64, name = "A", type = "companion"}]'}}
+; expected-error @+1 {{unapplied annotations with target '~NLAParse|NLAParse/dut:DUT/foobar:FooBar>clock' and payload '[{class = "sifive.enterprise.grandcentral.AugmentedGroundType", id = 1 : i64, target = []}, {class = "firrtl.transforms.DontTouchAnnotation", target = []}]'}}
+circuit NLAParse : %[[
+  {
+   "class": "sifive.enterprise.grandcentral.GrandCentralView$SerializedViewAnnotation", 
+    "companion": "~NLAParse|A_companion", 
+    "name": "A", 
+    "parent": "~NLAParse|DUT", 
+    "view": {
+      "class": "sifive.enterprise.grandcentral.AugmentedBundleType", 
+      "defName": "B", 
+      "elements": [
+        {
+          "name": "C", 
+          "tpe": {
+            "class": "sifive.enterprise.grandcentral.AugmentedBundleType", 
+            "defName": "D", 
+            "elements": [
+              {
+                "name": "clock", 
+                "tpe": {
+                  "class": "sifive.enterprise.grandcentral.AugmentedGroundType", 
+                  "ref": {
+                    "circuit": "NLAParse", 
+                    "component": [], 
+                    "module": "NLAParse", 
+                    "path": [
+                      {
+                        "_1": {
+                          "value": "dut"
+                        },
+                        "_2": {
+                          "value": "DUT"
+                        }
+                      },
+                      {
+                        "_1": {
+                          "value": "foobar"
+                        }, 
+                        "_2": {
+                          "value": "FooBar"
+                        }
+                      }
+                    ], 
+                    "ref": "clock"
+                  }
+                }
+              }
+            ]
+          }
+        }
+      ]
+    }
+  }
+]]
+
+  module FooBar :
+
+  module DUT :
+    inst foobar of FooBar
+
+  module NLAParse :
+    inst dut of DUT