[Moore] SymbolVisibility attribute support for SVModuleOp (#7278)

We can append the appropriate symbol visibility to svmoduleOp according
to the structures of instanceSymbol provided by the slang front-end. slang
provides a function to get the root of the design. Calling this method
could get all top-level instanceSymbols and help determine which ones
should be tagged. Note that the visibility attribute now used does not
contain the `nested`.

include/circt/Dialect/Moore/MooreOps.td

@@ -57,11 +57,14 @@ def SVModuleOp : MooreOp<"module", [
 
   let arguments = (ins
     SymbolNameAttr:$sym_name,
-    TypeAttrOf<ModuleType>:$module_type
+    TypeAttrOf<ModuleType>:$module_type,
+    OptionalAttr<StrAttr>:$sym_visibility
   );
   let regions = (region SizedRegion<1>:$bodyRegion);
   let hasCustomAssemblyFormat = 1;
 
+  let builders = [OpBuilder<
+    (ins "StringRef":$name, "hw::ModuleType":$type)>];
   let extraClassDeclaration = [{
     /// Return the `moore.output` op terminator of this module.
     OutputOp getOutputOp();

lib/Conversion/ImportVerilog/Structure.cpp

@@ -116,6 +116,9 @@ struct MemberVisitor {
     auto module = moduleLowering->op;
     auto moduleType = module.getModuleType();
 
+    // Set visibility attribute for instantiated module.
+    SymbolTable::setSymbolVisibility(module, SymbolTable::Visibility::Private);
+
     // Prepare the values that are involved in port connections. This creates
     // rvalues for input ports and appropriate lvalues for output, inout, and
     // ref ports. We also separate multi-ports into the individual underlying

lib/Dialect/Moore/MooreOps.cpp

@@ -25,19 +25,37 @@ using namespace mlir;
 // SVModuleOp
 //===----------------------------------------------------------------------===//
 
+void SVModuleOp::build(mlir::OpBuilder &builder, mlir::OperationState &state,
+                       llvm::StringRef name, hw::ModuleType type) {
+  state.addAttribute(SymbolTable::getSymbolAttrName(),
+                     builder.getStringAttr(name));
+  state.addAttribute(getModuleTypeAttrName(state.name), TypeAttr::get(type));
+  state.addRegion();
+}
+
 void SVModuleOp::print(OpAsmPrinter &p) {
   p << " ";
+
+  // Print the visibility of the module.
+  StringRef visibilityAttrName = SymbolTable::getVisibilityAttrName();
+  if (auto visibility = (*this)->getAttrOfType<StringAttr>(visibilityAttrName))
+    p << visibility.getValue() << ' ';
+
   p.printSymbolName(SymbolTable::getSymbolName(*this).getValue());
   hw::module_like_impl::printModuleSignatureNew(p, getBodyRegion(),
                                                 getModuleType(), {}, {});
-  p.printOptionalAttrDictWithKeyword(getOperation()->getAttrs(),
-                                     getAttributeNames());
   p << " ";
   p.printRegion(getBodyRegion(), /*printEntryBlockArgs=*/false,
                 /*printBlockTerminators=*/true);
+
+  p.printOptionalAttrDictWithKeyword(getOperation()->getAttrs(),
+                                     getAttributeNames());
 }
 
 ParseResult SVModuleOp::parse(OpAsmParser &parser, OperationState &result) {
+  // Parse the visibility attribute.
+  (void)mlir::impl::parseOptionalVisibilityKeyword(parser, result.attributes);
+
   // Parse the module name.
   StringAttr nameAttr;
   if (parser.parseSymbolName(nameAttr, getSymNameAttrName(result.name),

test/Conversion/ImportVerilog/basic.sv

@@ -13,10 +13,10 @@ endmodule
 // CHECK-LABEL: moore.module @NestedA() {
 // CHECK:         moore.instance "NestedB" @NestedB
 // CHECK:       }
-// CHECK-LABEL: moore.module @NestedB() {
+// CHECK-LABEL: moore.module private @NestedB() {
 // CHECK:         moore.instance "NestedC" @NestedC
 // CHECK:       }
-// CHECK-LABEL: moore.module @NestedC() {
+// CHECK-LABEL: moore.module private @NestedC() {
 // CHECK:       }
 module NestedA;
   module NestedB;
@@ -25,7 +25,7 @@ module NestedA;
   endmodule
 endmodule
 
-// CHECK-LABEL: moore.module @Child() {
+// CHECK-LABEL: moore.module private @Child() {
 // CHECK:       }
 module Child;
 endmodule
@@ -1073,7 +1073,7 @@ module PortsTop;
   PortsUnconnected p4(.a(), .b(x4), .c(), .d(y4), .e());
 endmodule
 
-// CHECK-LABEL: moore.module @PortsAnsi
+// CHECK-LABEL: moore.module private @PortsAnsi
 module PortsAnsi(
   // CHECK-SAME: in %a : !moore.l1
   input a,
@@ -1100,7 +1100,7 @@ module PortsAnsi(
   // CHECK: moore.output [[B_READ]] : !moore.l1
 endmodule
 
-// CHECK-LABEL: moore.module @PortsNonAnsi
+// CHECK-LABEL: moore.module private @PortsNonAnsi
 module PortsNonAnsi(a, b, c, d);
   // CHECK-SAME: in %a : !moore.l1
   input a;
@@ -1112,7 +1112,7 @@ module PortsNonAnsi(a, b, c, d);
   ref logic d;
 endmodule
 
-// CHECK-LABEL: moore.module @PortsExplicit
+// CHECK-LABEL: moore.module private @PortsExplicit
 module PortsExplicit(
   // CHECK-SAME: in %a0 : !moore.l1
   input .a0(x),
@@ -1141,7 +1141,7 @@ module PortsExplicit(
   // CHECK: moore.output [[B0]], [[X_READ]], [[B2]]
 endmodule
 
-// CHECK-LABEL: moore.module @MultiPorts
+// CHECK-LABEL: moore.module private @MultiPorts
 module MultiPorts(
   // CHECK-SAME: in %a0 : !moore.l1
   .a0(u[0]),
@@ -1184,7 +1184,7 @@ module MultiPorts(
   // CHECK: moore.output [[V1_READ]], [[C1_READ]]
 endmodule
 
-// CHECK-LABEL: moore.module @PortsUnconnected
+// CHECK-LABEL: moore.module private @PortsUnconnected
 module PortsUnconnected(
   // CHECK-SAME: in %a : !moore.l1
   input a,

test/Dialect/Moore/basic.mlir

@@ -103,36 +103,36 @@ moore.module @Module() {
   }
 }
 
-// CHECK-LABEL: func.func @Expressions
-func.func @Expressions(
-  // CHECK-SAME: [[A:%[^:]+]]: !moore.i32
-  // CHECK-SAME: [[B:%[^:]+]]: !moore.i32
-  %a: !moore.i32,
-  %b: !moore.i32,
-  // CHECK-SAME: [[C:%[^:]+]]: !moore.l32
-  // CHECK-SAME: [[D:%[^:]+]]: !moore.l3
-  %c: !moore.l32,
-  %d: !moore.l32,
-  // CHECK-SAME: [[X:%[^:]+]]: !moore.i1
-  %x: !moore.i1,
+// CHECK-LABEL: moore.module @Expressions
+moore.module @Expressions(
+  // CHECK-SAME: in [[A:%[^:]+]] : !moore.i32
+  // CHECK-SAME: in [[B:%[^:]+]] : !moore.i32
+  in %a: !moore.i32,
+  in %b: !moore.i32,
+  // CHECK-SAME: in [[C:%[^:]+]] : !moore.l32
+  // CHECK-SAME: in [[D:%[^:]+]] : !moore.l3
+  in %c: !moore.l32,
+  in %d: !moore.l32,
+  // CHECK-SAME: in [[X:%[^:]+]] : !moore.i1
+  in %x: !moore.i1,
 
-  // CHECK-SAME: [[ARRAY1:%[^:]+]]: !moore.uarray<4 x i8>
-  %array1: !moore.uarray<4 x i8>,
-  // CHECK-SAME: [[ARRAY2:%[^:]+]]: !moore.uarray<2 x uarray<4 x i8>>
-  %array2: !moore.uarray<2 x uarray<4 x i8>>,
+  // CHECK-SAME: in [[ARRAY1:%[^:]+]] : !moore.uarray<4 x i8>
+  in %array1: !moore.uarray<4 x i8>,
+  // CHECK-SAME: in [[ARRAY2:%[^:]+]] : !moore.uarray<2 x uarray<4 x i8>>
+  in %array2: !moore.uarray<2 x uarray<4 x i8>>,
 
-  // CHECK-SAME: [[REF_A:%[^:]+]]: !moore.ref<i32>
-  %refA: !moore.ref<i32>,
-  // CHECK-SAME: [[REF_B:%[^:]+]]: !moore.ref<i32>
-  %refB: !moore.ref<i32>,
-  // CHECK-SAME: [[REF_C:%[^:]+]]: !moore.ref<l32>
-  %refC: !moore.ref<l32>,
-  // CHECK-SAME: [[REF_D:%[^:]+]]: !moore.ref<l32>
-  %refD: !moore.ref<l32>,
-  // CHECK-SAME: [[REF_ARRAY1:%[^:]+]]: !moore.ref<uarray<4 x i8>>
-  %refArray1: !moore.ref<!moore.uarray<4 x i8>>,
-  // CHECK-SAME: [[REF_ARRAY2:%[^:]+]]: !moore.ref<uarray<2 x uarray<4 x i8>>>
-  %refArray2: !moore.ref<uarray<2 x uarray<4 x i8>>>
+  // CHECK-SAME: in [[REF_A:%[^:]+]] : !moore.ref<i32>
+  in %refA: !moore.ref<i32>,
+  // CHECK-SAME: in [[REF_B:%[^:]+]] : !moore.ref<i32>
+  in %refB: !moore.ref<i32>,
+  // CHECK-SAME: in [[REF_C:%[^:]+]] : !moore.ref<l32>
+  in %refC: !moore.ref<l32>,
+  // CHECK-SAME: in [[REF_D:%[^:]+]] : !moore.ref<l32>
+  in %refD: !moore.ref<l32>,
+  // CHECK-SAME: in [[REF_ARRAY1:%[^:]+]] : !moore.ref<uarray<4 x i8>>
+  in %refArray1: !moore.ref<!moore.uarray<4 x i8>>,
+  // CHECK-SAME: in [[REF_ARRAY2:%[^:]+]] : !moore.ref<uarray<2 x uarray<4 x i8>>>
+  in %refArray2: !moore.ref<uarray<2 x uarray<4 x i8>>>
 ) {
   // CHECK: moore.constant 0 : i32
   moore.constant 0 : i32
@@ -269,7 +269,7 @@ func.func @Expressions(
   } {
     moore.yield %b : i32
   }
-  return
+  moore.output
 }
 
 // CHECK-LABEL: moore.module @GraphRegion