[ImportVerilog] Incorporate block names to variable/instance names (#8680)

Use the names of surrounding generate blocks as a prefix for variable
and instance names. This makes the names match up more closely with how
existing EDA tools would label variables and instances embededded in
generate blocks.

Consider the following Verilog input:

```systemverilog
module Top;
  int x;
  begin : foo
    int y;
    for (genvar i = 2; i < 6; ++i) begin : bar
      int z;
    end
  end
endmodule
```

The ImportVerilog conversion pass will now produce the following
variables:

```mlir
%x = moore.variable : <i32>            // before: %x
%foo.y = moore.variable : <i32>        // before: %y
%foo.bar_2.z = moore.variable : <i32>  // before: %z
%foo.bar_3.z = moore.variable : <i32>  // before: %z_0
%foo.bar_4.z = moore.variable : <i32>  // before: %z_1
%foo.bar_5.z = moore.variable : <i32>  // before: %z_2
```

This closely matches names like `foo.bar[2].z` or `foo_bar_2_z` that
would be generated by common EDA tools. Ideally we would be able to use
a name like `foo.bar[2].z` directly, but ExportVerilog currently does
not support escaped identifiers, causing the name to be sanitized to
something like `foo_bar5B25D_z`.

By incorporating for-generate loop indices and block names into the
variable name, logical equivalence checking should become easier and the
signal names in Arcilator waveforms will match user expectations more
closely.

Fixes #8679.

lib/Conversion/ImportVerilog/Structure.cpp

@@ -228,9 +228,15 @@ static moore::NetKind convertNetKind(slang::ast::NetType::NetKind kind) {
 
 namespace {
 struct ModuleVisitor : public BaseVisitor {
-  using BaseVisitor::BaseVisitor;
   using BaseVisitor::visit;
 
+  // A prefix of block names such as `foo.bar.` to put in front of variable and
+  // instance names.
+  StringRef blockNamePrefix;
+
+  ModuleVisitor(Context &context, Location loc, StringRef blockNamePrefix = "")
+      : BaseVisitor(context, loc), blockNamePrefix(blockNamePrefix) {}
+
   // Skip ports which are already handled by the module itself.
   LogicalResult visit(const slang::ast::PortSymbol &) { return success(); }
   LogicalResult visit(const slang::ast::MultiPortSymbol &) { return success(); }
@@ -413,7 +419,8 @@ struct ModuleVisitor : public BaseVisitor {
     auto inputNames = builder.getArrayAttr(moduleType.getInputNames());
     auto outputNames = builder.getArrayAttr(moduleType.getOutputNames());
     auto inst = builder.create<moore::InstanceOp>(
-        loc, moduleType.getOutputTypes(), builder.getStringAttr(instNode.name),
+        loc, moduleType.getOutputTypes(),
+        builder.getStringAttr(Twine(blockNamePrefix) + instNode.name),
         FlatSymbolRefAttr::get(module.getSymNameAttr()), inputValues,
         inputNames, outputNames);
 
@@ -452,7 +459,7 @@ struct ModuleVisitor : public BaseVisitor {
 
     auto varOp = builder.create<moore::VariableOp>(
         loc, moore::RefType::get(cast<moore::UnpackedType>(loweredType)),
-        builder.getStringAttr(varNode.name), initial);
+        builder.getStringAttr(Twine(blockNamePrefix) + varNode.name), initial);
     context.valueSymbols.insert(&varNode, varOp);
     return success();
   }
@@ -479,7 +486,8 @@ struct ModuleVisitor : public BaseVisitor {
 
     auto netOp = builder.create<moore::NetOp>(
         loc, moore::RefType::get(cast<moore::UnpackedType>(loweredType)),
-        builder.getStringAttr(netNode.name), netkind, assignment);
+        builder.getStringAttr(Twine(blockNamePrefix) + netNode.name), netkind,
+        assignment);
     context.valueSymbols.insert(&netNode, netOp);
     return success();
   }
@@ -538,19 +546,55 @@ struct ModuleVisitor : public BaseVisitor {
 
   // Handle generate block.
   LogicalResult visit(const slang::ast::GenerateBlockSymbol &genNode) {
-    if (!genNode.isUninstantiated) {
-      for (auto &member : genNode.members()) {
-        if (failed(member.visit(ModuleVisitor(context, loc))))
-          return failure();
-      }
+    // Ignore uninstantiated blocks.
+    if (genNode.isUninstantiated)
+      return success();
+
+    // If the block has a name, add it to the list of block name prefices.
+    SmallString<64> prefixBuffer;
+    auto prefix = blockNamePrefix;
+    if (!genNode.name.empty()) {
+      prefixBuffer += blockNamePrefix;
+      prefixBuffer += genNode.name;
+      prefixBuffer += '.';
+      prefix = prefixBuffer;
     }
+
+    // Visit each member of the generate block.
+    for (auto &member : genNode.members())
+      if (failed(member.visit(ModuleVisitor(context, loc, prefix))))
+        return failure();
     return success();
   }
 
   // Handle generate block array.
   LogicalResult visit(const slang::ast::GenerateBlockArraySymbol &genArrNode) {
-    for (const auto *member : genArrNode.entries) {
-      if (failed(member->asSymbol().visit(ModuleVisitor(context, loc))))
+    // If the block has a name, add it to the list of block name prefices and
+    // prepare to append the array index and a `.` in each iteration.
+    SmallString<64> prefixBuffer;
+    if (!genArrNode.name.empty()) {
+      prefixBuffer += blockNamePrefix;
+      prefixBuffer += genArrNode.name;
+    }
+    auto prefixBufferBaseLen = prefixBuffer.size();
+
+    // Visit each iteration entry of the generate block.
+    for (const auto *entry : genArrNode.entries) {
+      // Append the index to the prefix if this block has a name.
+      auto prefix = blockNamePrefix;
+      if (prefixBufferBaseLen > 0) {
+        prefixBuffer.resize(prefixBufferBaseLen);
+        prefixBuffer += '_';
+        if (entry->arrayIndex)
+          prefixBuffer += entry->arrayIndex->toString();
+        else
+          Twine(entry->constructIndex).toVector(prefixBuffer);
+        prefixBuffer += '.';
+        prefix = prefixBuffer;
+      }
+
+      // Visit this iteration entry.
+      if (failed(entry->asSymbol().visit(ModuleVisitor(context, loc, prefix))))
         return failure();
     }
     return success();

test/Conversion/ImportVerilog/basic.sv

@@ -1869,6 +1869,43 @@ module GenerateConstructs;
   endgenerate
 endmodule
 
+// CHECK-LABEL: @UseGenerateBlockNameInVariables
+module UseGenerateBlockNameInVariables;
+  // CHECK: %x = moore.variable
+  int x;
+  begin : foo
+    // CHECK: %foo.y = moore.variable
+    int y;
+    for (genvar i = 2; i < 6; ++i) begin : bar
+      // CHECK: %foo.bar_2.z = moore.variable
+      // CHECK: %foo.bar_3.z = moore.variable
+      // CHECK: %foo.bar_4.z = moore.variable
+      // CHECK: %foo.bar_5.z = moore.variable
+      int z;
+    end
+  end
+endmodule
+
+// CHECK-LABEL: @UseGenerateBlockNameInInstances
+module UseGenerateBlockNameInInstances;
+  // CHECK: moore.instance "x" @Dummy
+  Dummy x();
+  begin : foo
+    // CHECK: moore.instance "foo.y" @Dummy
+    Dummy y();
+    for (genvar i = 2; i < 6; ++i) begin : bar
+      // CHECK: moore.instance "foo.bar_2.z" @Dummy
+      // CHECK: moore.instance "foo.bar_3.z" @Dummy
+      // CHECK: moore.instance "foo.bar_4.z" @Dummy
+      // CHECK: moore.instance "foo.bar_5.z" @Dummy
+      Dummy z();
+    end
+  end
+endmodule
+
+module Dummy;
+endmodule
+
 // Should accept and ignore empty packages.
 package Package;
   typedef logic [41:0] PackageType;