[ExportVerilog] Don't inline unpacked array assignments (#4548)

This changes emission style for unpacked array declaration. Verilator doesn't support initialization assignments for unpacked arrays, e.g:
```verilog
wire w[1:0] = '{0, 0};
```
This PR checks the value type and prevents inlining. Ideally it is more desirable to improve verilator but for now I want to avoid inlining unpacked arrays to declaration since it's just a tiny readability optimization. 

Fix #6363.

lib/Conversion/ExportVerilog/ExportVerilog.cpp

@@ -341,6 +341,13 @@ static Type stripUnpackedTypes(Type type) {
       .Default([](Type type) { return type; });
 }
 
+/// Return true if the type has a leading unpacked type.
+static bool hasLeadingUnpackedType(Type type) {
+  assert(isa<hw::InOutType>(type) && "inout type is expected");
+  auto elementType = cast<hw::InOutType>(type).getElementType();
+  return stripUnpackedTypes(elementType) != elementType;
+}
+
 /// Return true if type has a struct type as a subtype.
 static bool hasStructType(Type type) {
   return TypeSwitch<Type, bool>(type)
@@ -5855,8 +5862,11 @@ LogicalResult StmtEmitter::emitDeclaration(Operation *op) {
     }
 
     // Try inlining an assignment into declarations.
+    // FIXME: Unpacked array is not inlined since several tools doesn't support
+    // that syntax. See Issue 6363.
     if (isa<sv::WireOp, LogicOp>(op) &&
-        !op->getParentOp()->hasTrait<ProceduralRegion>()) {
+        !op->getParentOp()->hasTrait<ProceduralRegion>() &&
+        !hasLeadingUnpackedType(op->getResult(0).getType())) {
       // Get a single assignments if any.
       if (auto singleAssign = getSingleAssignAndCheckUsers<AssignOp>(op)) {
         auto *source = singleAssign.getSrc().getDefiningOp();
@@ -5877,7 +5887,10 @@ LogicalResult StmtEmitter::emitDeclaration(Operation *op) {
     }
 
     // Try inlining a blocking assignment to logic op declaration.
-    if (isa<LogicOp>(op) && op->getParentOp()->hasTrait<ProceduralRegion>()) {
+    // FIXME: Unpacked array is not inlined since several tools doesn't support
+    // that syntax. See Issue 6363.
+    if (isa<LogicOp>(op) && op->getParentOp()->hasTrait<ProceduralRegion>() &&
+        !hasLeadingUnpackedType(op->getResult(0).getType())) {
       // Get a single assignment which might be possible to inline.
       if (auto singleAssign = getSingleAssignAndCheckUsers<BPAssignOp>(op)) {
         // It is necessary for the assignment to dominate users of the op.

test/Conversion/ExportVerilog/sv-dialect.mlir

@@ -521,7 +521,8 @@ hw.module @reg_0(in %in4: i4, in %in8: i8, in %in8_2: i8, out a: i8, out b: i8)
 
   %unpacked_array = sv.unpacked_array_create %in8, %in8_2 : (i8, i8) -> !hw.uarray<2xi8>
   %unpacked_wire = sv.wire : !hw.inout<uarray<2xi8>>
-  // CHECK: wire [7:0] unpacked_wire[0:1] = '{in8_2, in8};
+  // CHECK: wire [7:0] unpacked_wire[0:1];
+  // CHECK-NEXT: assign unpacked_wire = '{in8_2, in8};
   sv.assign %unpacked_wire, %unpacked_array: !hw.uarray<2xi8>
 
   // CHECK-NEXT: assign a = myReg;
@@ -1111,6 +1112,23 @@ hw.module @DontDuplicateSideEffectingVerbatim() {
   }
 }
 
+// Issue 6363
+// CHECK-LABEL: module DontInlineAssignmentForUnpackedArrays(
+hw.module @DontInlineAssignmentForUnpackedArrays(in %a: !hw.uarray<2xi1>) {
+// CHECK:      wire w[0:1];
+// CHECK-NEXT: assign w = a;
+   %w = sv.wire  : !hw.inout<uarray<2xi1>>
+   sv.assign %w, %a : !hw.uarray<2xi1>
+   // CHECK: logic u[0:1];
+   // CHECK-NEXT: u = a;
+   sv.initial {
+    %u = sv.logic  : !hw.inout<uarray<2xi1>>
+    sv.bpassign %u, %a : !hw.uarray<2xi1>
+   }
+
+   hw.output
+}
+
 hw.generator.schema @verbatim_schema, "Simple", ["ports", "write_latency", "read_latency"]
 hw.module.extern @verbatim_inout_2 ()
 // CHECK-LABEL: module verbatim_M1(
@@ -1789,7 +1807,8 @@ sv.func private @open_array(in %array : !sv.open_uarray<i8>)
 sv.func.dpi.import @open_array
 
 // CHECK-LABEL: test_open_array
-// CHECK: wire [7:0] _GEN[0:1] = '{in_0, in_1};
+// CHECK: wire [7:0] _GEN[0:1];
+// CHECK-NEXT: assign _GEN = '{in_0, in_1};
 // CHECK-NEXT: always @(posedge clock)
 // CHECK-NEXT:   open_array(_GEN);
 hw.module @test_open_array(in %clock : i1, in %in_0 : i8, in %in_1 : i8) {

test/Conversion/ExportVerilog/verilog-basic.mlir

@@ -489,7 +489,8 @@ hw.module @ArrayParamsInst() {
     uarr: %uarr : !hw.uarray<2 x i8>) -> ()
 }
 // CHECK:       wire [1:0][7:0] [[G0:_.*]] = '{8'h1, 8'h2};
-// CHECK:       wire [7:0]      [[G1:_.*]][0:1] = '{8'h1, 8'h2};
+// CHECK:       wire [7:0]      [[G1:_.*]][0:1];
+// CHECK:       assign [[G1]] = '{8'h1, 8'h2};
 // CHECK:       ArrayParams #(
 // CHECK:         .param(2)
 // CHECK:       ) arrays (

test/firtool/dpi.fir

@@ -33,7 +33,8 @@ circuit DPI:
 ; CHECK-LABEL: module DPI(
 ; CHECK:        logic [7:0] [[TMP:_.+]];
 ; CHECK-NEXT:   reg   [7:0] [[RESULT1:_.+]];
-; CHECK-NEXT:   wire [7:0] [[OPEN_ARRAY:_.+]][0:1] = '{in_0, in_1};
+; CHECK-NEXT:   wire [7:0] [[OPEN_ARRAY:_.+]][0:1];
+; CHECK-NEXT:   assign [[OPEN_ARRAY]] = '{in_0, in_1};
 ; CHECK-NEXT:   always @(posedge clock) begin
 ; CHECK-NEXT:     if (enable) begin
 ; CHECK-NEXT:       clocked_result(in_0, in_1, [[TMP]]);