[FIRRTL] Fix SRAM instance paths in OMIR (llvm#2039)

Fix an issue in `EmitOMIR` where the instance paths of SRAMs would be
incorrectly rendered as `OMMemberReferenceTarget` instead of the correct
`OMMemberInstanceTarget`. This also includes figuring out the module
name that the final SRAM is going to have, which in the case of a
`firrtl.mem` is a bit tricky/fragile: the actual module name for the
memory is only generated after lowering to the HW dialect, at which
point the OMIR has long been emitted. This change "predicts" the name
that the memory is going to have by querying `getFirMemoryName()`, which
looks like it's going to be accurate in most cases we currently care
about.

In the long run, we'll want better ways to encode this formulation of an
instance path and ideally dealy it until `ExportVerilog` time.

Co-authored-by: Schuyler Eldridge <schuyler.eldridge@gmail.com>

lib/Dialect/FIRRTL/Transforms/EmitOMIR.cpp

@@ -571,6 +571,14 @@ void EmitOMIRPass::emitTrackedTarget(DictionaryAttr node,
   }
   auto tracker = trackerIt->second;
 
+  // In case this is an `OMMemberTarget`, handle the case where the component
+  // used to be a "reference target" (wire, register, memory, node) when the
+  // OMIR was read in, but has been change to an "instance target" during the
+  // execution of the compiler. This mainly occurs during mapping of
+  // `firrtl.mem` operations to a corresponding `firrtl.instance`.
+  if (type == "OMMemberReferenceTarget" && isa<InstanceOp, MemOp>(tracker.op))
+    type = "OMMemberInstanceTarget";
+
   // Serialize the target circuit first.
   SmallString<64> target(type);
   target.append(":~");
@@ -634,8 +642,30 @@ void EmitOMIRPass::emitTrackedTarget(DictionaryAttr node,
     return jsonStream.value("<error>");
   }
   if (!componentName.empty()) {
-    target.push_back('>');
-    target.append(componentName);
+    // Check if the targeted component is going to be emitted as an instance.
+    // This is trivially the case for `InstanceOp`s, but also for `MemOp`s that
+    // get converted to an instance during lowering to HW dialect and generator
+    // callout.
+    [&] {
+      if (type == "OMMemberInstanceTarget") {
+        if (auto instOp = dyn_cast<InstanceOp>(tracker.op)) {
+          target.push_back('/');
+          target.append(instOp.name());
+          target.push_back(':');
+          target.append(addSymbol(instOp.moduleNameAttr()));
+          return;
+        }
+        if (auto memOp = dyn_cast<MemOp>(tracker.op)) {
+          target.push_back('/');
+          target.append(memOp.name());
+          target.push_back(':');
+          target.append(memOp.getSummary().getFirMemoryName());
+          return;
+        }
+      }
+      target.push_back('>');
+      target.append(componentName);
+    }();
   }
 
   jsonStream.value(target);

test/Dialect/FIRRTL/SFCTests/emit-omir.fir

@@ -41,7 +41,7 @@ circuit Foo : %[[
         "id": "OMID:2",
         "fields": [
           {"info": "", "name": "omType", "value": ["OMString:OMLazyModule", "OMString:OMSRAM"]},
-          {"info": "", "name": "instancePath", "value": "OMMemberReferenceTarget:~Foo|Bar>mem1"}
+          {"info": "", "name": "instancePath", "value": "OMMemberInstanceTarget:~Foo|Bar>mem1"}
         ]
       },
       {
@@ -110,8 +110,8 @@ circuit Foo : %[[
 
 ; CHECK:       "id": "OMID:2"
 ; CHECK:       "name": "instancePath"
-; CHECK-NEXT:  "value": "OMMemberReferenceTarget:~Foo|Foo/bar:Bar>mem1"
+; CHECK-NEXT:  "value": "OMMemberInstanceTarget:~Foo|Foo/bar:Bar/mem1:MySRAM"
 
 ; CHECK:       "id": "OMID:3"
 ; CHECK:       "name": "instancePath"
-; CHECK-NEXT:  "value": "OMMemberReferenceTarget:~Foo|Foo/bar:Bar>mem2"
+; CHECK-NEXT:  "value": "OMMemberInstanceTarget:~Foo|Foo/bar:Bar/mem2:FIRRTLMem_{{[^"]+}}"

test/Dialect/FIRRTL/emit-omir.mlir

@@ -246,6 +246,10 @@ firrtl.circuit "SRAMPaths" attributes {annotations = [{
       id = "OMID:0",
       fields = {
         omType = {info = #loc, index = 0, value = ["OMString:OMLazyModule", "OMString:OMSRAM"]},
+        // We purposefully pick the wrong `OMMemberTarget` here, to check that
+        // it actually gets emitted as a `OMMemberInstanceTarget`. These can
+        // change back and forth as the FIRRTL passes work on the IR, and the
+        // OMIR output should reflect the final target.
         instancePath = {info = #loc, index = 1, value = {omir.tracker, id = 0, type = "OMMemberReferenceTarget"}}
       }
     },
@@ -294,13 +298,13 @@ firrtl.circuit "SRAMPaths" attributes {annotations = [{
 // CHECK-SAME:      \22OMString:OMSRAM\22\0A
 // CHECK-SAME:    ]
 // CHECK-SAME:    \22name\22: \22instancePath\22
-// CHECK-SAME:    \22value\22: \22OMMemberReferenceTarget:~SRAMPaths|{{[{][{]0[}][}]}}/sub:{{[{][{]1[}][}]}}>mem1\22
+// CHECK-SAME:    \22value\22: \22OMMemberInstanceTarget:~SRAMPaths|{{[{][{]0[}][}]}}/sub:{{[{][{]1[}][}]}}/mem1:{{[{][{]2[}][}]}}\22
 // CHECK-SAME:  \22id\22: \22OMID:1\22
 // CHECK-SAME:    \22name\22: \22omType\22
 // CHECK-SAME:    \22value\22: [
 // CHECK-SAME:      \22OMString:OMLazyModule\22
 // CHECK-SAME:      \22OMString:OMSRAM\22\0A
 // CHECK-SAME:    ]
 // CHECK-SAME:    \22name\22: \22instancePath\22
-// CHECK-SAME:    \22value\22: \22OMMemberReferenceTarget:~SRAMPaths|{{[{][{]0[}][}]}}/sub:{{[{][{]1[}][}]}}>mem2\22
-// CHECK-SAME:  symbols = [@SRAMPaths, @Submodule]
+// CHECK-SAME:    \22value\22: \22OMMemberInstanceTarget:~SRAMPaths|{{[{][{]0[}][}]}}/sub:{{[{][{]1[}][}]}}/mem2:FIRRTLMem_{{[^\\]+}}\22
+// CHECK-SAME:  symbols = [@SRAMPaths, @Submodule, @MySRAM]