From 48421e4ce0e0fa0af7d2e29deb985645c8ad1f81 Mon Sep 17 00:00:00 2001 From: Nick Brereton Date: Fri, 10 Jul 2026 17:02:03 -0400 Subject: [PATCH 1/2] Fix streaming concat as output-port lvalue into unpacked struct --- src/V3Inst.cpp | 10 +- src/V3Width.cpp | 140 +++++++++--------- src/V3Width.h | 2 + .../t/t_stream_unpacked_struct_port.py | 18 +++ .../t/t_stream_unpacked_struct_port.v | 116 +++++++++++++++ 5 files changed, 217 insertions(+), 69 deletions(-) create mode 100755 test_regress/t/t_stream_unpacked_struct_port.py create mode 100644 test_regress/t/t_stream_unpacked_struct_port.v diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index befe8b05b..9201f95f4 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -26,6 +26,7 @@ #include "V3Inst.h" #include "V3Const.h" +#include "V3Width.h" VL_DEFINE_DEBUG_FUNCTIONS; @@ -909,9 +910,14 @@ public: << pinexprp->width()); rhsp = extendOrSel(pinp->fileline(), rhsp, pinVarp); pinp->exprp(new AstVarRef{newvarp->fileline(), newvarp, VAccess::WRITE}); - AstNodeExpr* const rhsSelp = extendOrSel(pinp->fileline(), rhsp, pinexprp); markContinuousLhs(pinexprp); - assignp = new AstAssignW{pinp->fileline(), pinexprp, rhsSelp}; + if (VN_IS(pinexprp, NodeStream)) { + assignp = new AstAssignW{pinp->fileline(), pinexprp, rhsp}; + V3Width::streamAssignLowerEdit(assignp); + } else { + AstNodeExpr* const rhsSelp = extendOrSel(pinp->fileline(), rhsp, pinexprp); + assignp = new AstAssignW{pinp->fileline(), pinexprp, rhsSelp}; + } } else { // V3 width should have range/extended to make the widths correct newvarp->isContinuously(true); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index ac0cfdcf5..109f349c7 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -284,9 +284,6 @@ class WidthVisitor final : public VNVisitor { nodep->findLogicDType(unpackBits, unpackMinBits, VSigning::UNSIGNED)}); } } - static bool lowerAsFixedAggregate(const AstNodeDType* const dtypep) { - return dtypep->isStreamableFixedAggregate() && dtypep->containsUnpackedStruct(); - } // When fromp() is a DType (e.g. unlinked RefDType), resolve through // the ref chain; when it's an expression, dtypep() is already resolved. static AstNodeDType* fromDTypep(AstNode* fromp) { @@ -6401,70 +6398,8 @@ class WidthVisitor final : public VNVisitor { userIterateAndNext(nodep->rhsp(), WidthVP{nodep->dtypep(), PRELIM}.p()); // // UINFOTREE(1, nodep, "", "assign"); - AstNodeDType* lhsDTypep - = nodep->lhsp()->dtypep(); // Note we use rhsp for context determined - - // Check width of stream and wrap if needed - if (AstNodeStream* const streamp = VN_CAST(nodep->rhsp(), NodeStream)) { - AstNodeDType* const lhsDTypeSkippedRefp = lhsDTypep->skipRefp(); - const int lwidth = lhsDTypeSkippedRefp->widthStream(); - const int rwidth = streamp->lhsp()->dtypep()->skipRefp()->widthStream(); - if (lwidth != 0 && lwidth < rwidth) { - nodep->v3widthWarn(lwidth, rwidth, - "Target fixed size variable (" - << lwidth << " bits) is narrower than the stream (" - << rwidth << " bits) (IEEE 1800-2023 11.4.14)"); - } - if (VN_IS(streamp->lhsp()->dtypep()->skipRefp(), QueueDType) - && !VN_IS(nodep->lhsp()->dtypep()->skipRefp(), QueueDType)) { - const int queueElementSize = streamp->lhsp()->dtypep()->subDTypep()->width(); - UASSERT_OBJ(queueElementSize <= lwidth, nodep, "LHS < RHS"); - } - if (VN_IS(lhsDTypeSkippedRefp, UnpackArrayDType) - || lowerAsFixedAggregate(lhsDTypeSkippedRefp)) { - streamp->unlinkFrBack(); - nodep->rhsp(new AstCvtPackedToArray{streamp->fileline(), streamp, - lhsDTypeSkippedRefp}); - } - } - if (AstNodeStream* const streamp = VN_CAST(nodep->lhsp(), NodeStream)) { - const AstNodeDType* const rhsDTypep = nodep->rhsp()->dtypep()->skipRefp(); - AstNodeDType* const lhsStreamDTypep = streamp->lhsp()->dtypep()->skipRefp(); - const int lwidth = lhsStreamDTypep->widthStream(); - const int rwidth = rhsDTypep->widthStream(); - if (rwidth != 0 && rwidth < lwidth) { - nodep->v3widthWarn(lwidth, rwidth, - "Stream target requires " - << lwidth - << " bits, but source expression only provides " - << rwidth << " bits (IEEE 1800-2023 11.4.14.3)"); - } - if (lowerAsFixedAggregate(lhsStreamDTypep)) { - AstNodeExpr* const streamExprp = nodep->lhsp()->unlinkFrBack(); - AstNodeExpr* const dstp = streamp->lhsp()->unlinkFrBack(); - AstNodeExpr* srcp = nodep->rhsp()->unlinkFrBack(); - if (VN_IS(streamp, StreamL)) { - streamp->lhsp(srcp); - streamp->dtypeSetLogicUnsized(srcp->width(), srcp->widthMin(), - VSigning::UNSIGNED); - srcp = streamExprp; - } else { - if (srcp->width() > lwidth) { - srcp = new AstSel{streamp->fileline(), srcp, srcp->width() - lwidth, - lwidth}; - } - VL_DO_DANGLING(pushDeletep(streamExprp), streamExprp); - } - nodep->lhsp(dstp); - nodep->rhsp(new AstCvtPackedToArray{srcp->fileline(), srcp, lhsStreamDTypep}); - nodep->dtypeFrom(dstp); - lhsDTypep = nodep->lhsp()->dtypep(); - } else if (VN_IS(rhsDTypep, UnpackArrayDType)) { - AstNodeExpr* const rhsp = nodep->rhsp()->unlinkFrBack(); - nodep->rhsp( - new AstCvtArrayToPacked{rhsp->fileline(), rhsp, streamp->dtypep()}); - } - } + streamAssignLower(nodep); + AstNodeDType* const lhsDTypep = nodep->lhsp()->dtypep(); // IEEE 1800-2023 7.6: For unpacked arrays to be assignment compatible, // the element types shall be equivalent (IEEE 1800-2023 6.22.2). @@ -10282,6 +10217,71 @@ public: AstNode* mainAcceptEdit(AstNode* nodep) { return userIterateSubtreeReturnEdits(nodep, WidthVP{SELF, BOTH}.p()); } + static bool lowerAsFixedAggregate(const AstNodeDType* const dtypep) { + return dtypep->isStreamableFixedAggregate() && dtypep->containsUnpackedStruct(); + } + void streamAssignLower(AstNodeAssign* nodep) { + AstNodeDType* lhsDTypep = nodep->lhsp()->dtypep(); + // Check width of stream and wrap if needed + if (AstNodeStream* const streamp = VN_CAST(nodep->rhsp(), NodeStream)) { + AstNodeDType* const lhsDTypeSkippedRefp = lhsDTypep->skipRefp(); + const int lwidth = lhsDTypeSkippedRefp->widthStream(); + const int rwidth = streamp->lhsp()->dtypep()->skipRefp()->widthStream(); + if (lwidth != 0 && lwidth < rwidth) { + nodep->v3widthWarn(lwidth, rwidth, + "Target fixed size variable (" + << lwidth << " bits) is narrower than the stream (" + << rwidth << " bits) (IEEE 1800-2023 11.4.14)"); + } + if (VN_IS(streamp->lhsp()->dtypep()->skipRefp(), QueueDType) + && !VN_IS(nodep->lhsp()->dtypep()->skipRefp(), QueueDType)) { + const int queueElementSize = streamp->lhsp()->dtypep()->subDTypep()->width(); + UASSERT_OBJ(queueElementSize <= lwidth, nodep, "LHS < RHS"); + } + if (VN_IS(lhsDTypeSkippedRefp, UnpackArrayDType) + || lowerAsFixedAggregate(lhsDTypeSkippedRefp)) { + streamp->unlinkFrBack(); + nodep->rhsp(new AstCvtPackedToArray{streamp->fileline(), streamp, + lhsDTypeSkippedRefp}); + } + } + if (AstNodeStream* const streamp = VN_CAST(nodep->lhsp(), NodeStream)) { + const AstNodeDType* const rhsDTypep = nodep->rhsp()->dtypep()->skipRefp(); + AstNodeDType* const lhsStreamDTypep = streamp->lhsp()->dtypep()->skipRefp(); + const int lwidth = lhsStreamDTypep->widthStream(); + const int rwidth = rhsDTypep->widthStream(); + if (rwidth != 0 && rwidth < lwidth) { + nodep->v3widthWarn(lwidth, rwidth, + "Stream target requires " + << lwidth + << " bits, but source expression only provides " << rwidth + << " bits (IEEE 1800-2023 11.4.14.3)"); + } + if (lowerAsFixedAggregate(lhsStreamDTypep)) { + AstNodeExpr* const streamExprp = nodep->lhsp()->unlinkFrBack(); + AstNodeExpr* const dstp = streamp->lhsp()->unlinkFrBack(); + AstNodeExpr* srcp = nodep->rhsp()->unlinkFrBack(); + if (VN_IS(streamp, StreamL)) { + streamp->lhsp(srcp); + streamp->dtypeSetLogicUnsized(srcp->width(), srcp->widthMin(), + VSigning::UNSIGNED); + srcp = streamExprp; + } else { + if (srcp->width() > lwidth) { + srcp = new AstSel{streamp->fileline(), srcp, srcp->width() - lwidth, + lwidth}; + } + VL_DO_DANGLING(pushDeletep(streamExprp), streamExprp); + } + nodep->lhsp(dstp); + nodep->rhsp(new AstCvtPackedToArray{srcp->fileline(), srcp, lhsStreamDTypep}); + nodep->dtypeFrom(dstp); + } else if (VN_IS(rhsDTypep, UnpackArrayDType)) { + AstNodeExpr* const rhsp = nodep->rhsp()->unlinkFrBack(); + nodep->rhsp(new AstCvtArrayToPacked{rhsp->fileline(), rhsp, streamp->dtypep()}); + } + } + } ~WidthVisitor() override = default; }; @@ -10311,6 +10311,12 @@ AstNode* V3Width::widthParamsEdit(AstNode* nodep) { return nodep; } +void V3Width::streamAssignLowerEdit(AstNodeAssign* nodep) { + UINFO(4, __FUNCTION__ << ": " << nodep); + WidthVisitor visitor{false, false}; + visitor.streamAssignLower(nodep); +} + //! Single node parameter propagation for generate blocks. //! Smaller step... Only do a single node for parameter propagation //! If we are inside a generated "if", "case" or "for", we don't want to diff --git a/src/V3Width.h b/src/V3Width.h index 9634ff7a8..5bd305936 100644 --- a/src/V3Width.h +++ b/src/V3Width.h @@ -22,6 +22,7 @@ class AstNetlist; class AstNode; +class AstNodeAssign; class AstNodeDType; //============================================================================ @@ -32,6 +33,7 @@ public: static AstNode* widthParamsEdit(AstNode* nodep) VL_MT_DISABLED; static AstNode* widthGenerateParamsEdit(AstNode* nodep) VL_MT_DISABLED; static AstNode* selectNonConstantRecurse(AstNode* nodep, bool inSel = false) VL_MT_DISABLED; + static void streamAssignLowerEdit(AstNodeAssign* nodep) VL_MT_DISABLED; // For use only in WidthVisitor // Replace AstSelBit, etc with AstSel/AstArraySel diff --git a/test_regress/t/t_stream_unpacked_struct_port.py b/test_regress/t/t_stream_unpacked_struct_port.py new file mode 100755 index 000000000..8a938befd --- /dev/null +++ b/test_regress/t/t_stream_unpacked_struct_port.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of either the GNU Lesser General Public License Version 3 +# or the Perl Artistic License Version 2.0. +# SPDX-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_stream_unpacked_struct_port.v b/test_regress/t/t_stream_unpacked_struct_port.v new file mode 100644 index 000000000..7d8b5e7fe --- /dev/null +++ b/test_regress/t/t_stream_unpacked_struct_port.v @@ -0,0 +1,116 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of either the GNU Lesser General Public License Version 3 +// or the Perl Artistic License Version 2.0. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +// Ref. to IEEE 1800-2023 11.4.14 +// +// A streaming concatenation used as the lvalue of a module output-port +// connection, targeting an unpacked struct. This form is lowered by +// V3Inst::pinReconnectSimple (after V3Width), which must apply the same +// fixed-aggregate stream lowering as a normal streaming assignment. Without +// that lowering the port assign const-folds to a zero-width, driverless +// assign and the target reads as a constant 0 (and generates invalid C++). + +module t( /*AUTOARG*/ + // Inputs + clk +); + input clk; + + `define checkh(gotv, expv) \ + do if ((gotv) !== (expv)) begin \ + $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__, `__LINE__, (gotv), (expv)); \ + $stop; \ + end while (0); + + typedef struct packed { + logic [7:0] a; + logic [7:0] b; + } inner_t; + + // Unpacked struct, $bits == 40 + typedef struct { + inner_t x; + logic [15:0] c; + logic [7:0] d; + } order_t; + + // Unpacked array, also $bits == 40 (so producer width OW is shared). + typedef byte arr_t[5]; + + localparam int OW = $bits(order_t); + + integer cyc = 0; + reg [63:0] crc = 64'h5aef0c8d_d70a4497; + wire [OW-1:0] src = crc[OW-1:0]; + + // Path under test: streaming concat as an output-port lvalue, into an + // unpacked struct (needs CvtPackedToArray) and an unpacked array (does not). + order_t out_r; // right-stream {>>{}} + order_t out_l; // left-stream {<<8{}} + arr_t arr_r; + arr_t arr_l; + producer #(.W(OW)) u_r (.din(src), .dout({>>{out_r}})); + producer #(.W(OW)) u_l (.din(src), .dout({<<8{out_l}})); + producer #(.W(OW)) u_ar (.din(src), .dout({>>{arr_r}})); + producer #(.W(OW)) u_al (.din(src), .dout({<<8{arr_l}})); + + // Reference: plain packed port + separate streaming unpack assign. + wire [OW-1:0] pack_r; + wire [OW-1:0] pack_l; + order_t ref_r, ref_l; + producer #(.W(OW)) u_pr (.din(src), .dout(pack_r)); + producer #(.W(OW)) u_pl (.din(src), .dout(pack_l)); + assign ref_r = {>>{pack_r}}; + assign ref_l = {<<8{pack_l}}; + + // Packed views (streaming into a packed wire is the legal direction) so the + // checks compare plain vectors rather than unpacked structs/arrays. + wire [OW-1:0] out_r_bits = {>>{out_r}}; + wire [OW-1:0] out_l_bits = {<<8{out_l}}; + wire [OW-1:0] arr_r_bits = {>>{arr_r}}; + wire [OW-1:0] arr_l_bits = {<<8{arr_l}}; + wire [OW-1:0] ref_r_bits = {>>{ref_r}}; + wire [OW-1:0] ref_l_bits = {<<8{ref_l}}; + + always @(posedge clk) begin + cyc <= cyc + 1; + crc <= {crc[62:0], crc[63] ^ crc[2] ^ crc[0]}; + if (cyc > 1) begin + // The target has a real driver (not folded to 0) and round-trips. + `checkh(out_r_bits, src); + `checkh(out_l_bits, src); + `checkh(arr_r_bits, src); + `checkh(arr_l_bits, src); + // Output-port streaming lvalue matches the reference unpack. + `checkh(out_r_bits, ref_r_bits); + `checkh(out_l_bits, ref_l_bits); + if (out_r !== ref_r) begin + $write("%%Error: %s:%0d: out_r struct mismatch\n", `__FILE__, `__LINE__); + $stop; + end + if (out_l !== ref_l) begin + $write("%%Error: %s:%0d: out_l struct mismatch\n", `__FILE__, `__LINE__); + $stop; + end + end + if (cyc == 20) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module producer #( + parameter int W = 1 +) ( + input logic [W-1:0] din, + output logic [W-1:0] dout +); + assign dout = din; +endmodule From a79701134813a7f28ec374108407610a36d9e0d8 Mon Sep 17 00:00:00 2001 From: github action Date: Fri, 10 Jul 2026 21:07:06 +0000 Subject: [PATCH 2/2] Apply 'make format' [ci skip] --- src/V3Width.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 109f349c7..112caceb3 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -10241,8 +10241,8 @@ public: if (VN_IS(lhsDTypeSkippedRefp, UnpackArrayDType) || lowerAsFixedAggregate(lhsDTypeSkippedRefp)) { streamp->unlinkFrBack(); - nodep->rhsp(new AstCvtPackedToArray{streamp->fileline(), streamp, - lhsDTypeSkippedRefp}); + nodep->rhsp( + new AstCvtPackedToArray{streamp->fileline(), streamp, lhsDTypeSkippedRefp}); } } if (AstNodeStream* const streamp = VN_CAST(nodep->lhsp(), NodeStream)) { @@ -10253,9 +10253,8 @@ public: if (rwidth != 0 && rwidth < lwidth) { nodep->v3widthWarn(lwidth, rwidth, "Stream target requires " - << lwidth - << " bits, but source expression only provides " << rwidth - << " bits (IEEE 1800-2023 11.4.14.3)"); + << lwidth << " bits, but source expression only provides " + << rwidth << " bits (IEEE 1800-2023 11.4.14.3)"); } if (lowerAsFixedAggregate(lhsStreamDTypep)) { AstNodeExpr* const streamExprp = nodep->lhsp()->unlinkFrBack();