From 15a5c99ce88940ac2b42c47906707590ed9f5691 Mon Sep 17 00:00:00 2001 From: Hideto Ueno Date: Fri, 16 Aug 2024 04:59:15 -0700 Subject: [PATCH] [ExportVerilog] Don't inline unpacked array assignments --- .../ExportVerilog/ExportVerilog.cpp | 17 ++++++++++++-- test/Conversion/ExportVerilog/sv-dialect.mlir | 23 +++++++++++++++++-- .../ExportVerilog/verilog-basic.mlir | 3 ++- test/firtool/dpi.fir | 3 ++- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/lib/Conversion/ExportVerilog/ExportVerilog.cpp b/lib/Conversion/ExportVerilog/ExportVerilog.cpp index 74e5760ccae1..a67d02581c64 100644 --- a/lib/Conversion/ExportVerilog/ExportVerilog.cpp +++ b/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(type) && "inout type is expected"); + auto elementType = cast(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) @@ -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(op) && - !op->getParentOp()->hasTrait()) { + !op->getParentOp()->hasTrait() && + !hasLeadingUnpackedType(op->getResult(0).getType())) { // Get a single assignments if any. if (auto singleAssign = getSingleAssignAndCheckUsers(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(op) && op->getParentOp()->hasTrait()) { + // FIXME: Unpacked array is not inlined since several tools doesn't support + // that syntax. See Issue 6363. + if (isa(op) && op->getParentOp()->hasTrait() && + !hasLeadingUnpackedType(op->getResult(0).getType())) { // Get a single assignment which might be possible to inline. if (auto singleAssign = getSingleAssignAndCheckUsers(op)) { // It is necessary for the assignment to dominate users of the op. diff --git a/test/Conversion/ExportVerilog/sv-dialect.mlir b/test/Conversion/ExportVerilog/sv-dialect.mlir index 3d75fd441e62..8146d9dfc231 100644 --- a/test/Conversion/ExportVerilog/sv-dialect.mlir +++ b/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> - // 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> + sv.assign %w, %a : !hw.uarray<2xi1> + // CHECK: logic u[0:1]; + // CHECK-NEXT: u = a; + sv.initial { + %u = sv.logic : !hw.inout> + 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) 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) { diff --git a/test/Conversion/ExportVerilog/verilog-basic.mlir b/test/Conversion/ExportVerilog/verilog-basic.mlir index 2f28b0e88c04..55586662ac4f 100644 --- a/test/Conversion/ExportVerilog/verilog-basic.mlir +++ b/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 ( diff --git a/test/firtool/dpi.fir b/test/firtool/dpi.fir index ce47feb2529e..f4bf89d6a541 100644 --- a/test/firtool/dpi.fir +++ b/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]]);