From 7655ccf400c57505f326c056da19cc64969e3831 Mon Sep 17 00:00:00 2001 From: JAEUK LEE Date: Wed, 8 Jul 2026 19:01:51 +0900 Subject: [PATCH] Use width context for stream concat checks --- src/V3Width.cpp | 114 ++++++++++++++---------- test_regress/t/t_display_stream_bad.out | 5 ++ test_regress/t/t_display_stream_bad.v | 2 + 3 files changed, 76 insertions(+), 45 deletions(-) diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 46fc29914..4befae6ed 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -109,6 +109,12 @@ std::ostream& operator<<(std::ostream& str, const Determ& rhs) { return str << s_det[rhs]; } +enum StreamUse : uint8_t { + STREAM_USE_NONE, + STREAM_USE_ASSIGN, + STREAM_USE_CAST, +}; + #define v3widthWarn(lhs, rhs, msg) \ v3warnCode(((lhs) < (rhs) ? V3ErrorCode::WIDTHTRUNC \ : (lhs) > (rhs) ? V3ErrorCode::WIDTHEXPAND \ @@ -122,21 +128,27 @@ class WidthVP final { // Parameters to pass down hierarchy with visit functions. AstNodeDType* const m_dtypep; // Parent's data type to resolve to const Stage m_stage; // If true, report errors + const StreamUse m_streamUse; // Whether current expression may be a stream concat + public: - WidthVP(AstNodeDType* dtypep, Stage stage) + WidthVP(AstNodeDType* dtypep, Stage stage, StreamUse streamUse = STREAM_USE_NONE) : m_dtypep{dtypep} - , m_stage{stage} { + , m_stage{stage} + , m_streamUse{streamUse} { // Prelim doesn't look at assignments, so shouldn't need a dtype, // however AstPattern uses them } - WidthVP(Determ determ, Stage stage) + WidthVP(Determ determ, Stage stage, StreamUse streamUse = STREAM_USE_NONE) : m_dtypep{nullptr} - , m_stage{stage} { + , m_stage{stage} + , m_streamUse{streamUse} { if (determ != SELF && stage != PRELIM) v3fatalSrc("Context-determined width request only allowed as prelim step"); } WidthVP* p() { return this; } bool selfDtm() const { return m_dtypep == nullptr; } + StreamUse streamUse() const { return m_streamUse; } + bool streamUseAllowed() const { return m_streamUse != STREAM_USE_NONE; } AstNodeDType* dtypep() const { // Detect where overrideDType is probably the intended call UASSERT(m_dtypep, "Width dtype request on self-determined or preliminary VUP"); @@ -164,12 +176,17 @@ public: bool final() const { return m_stage & FINAL; } void dump(std::ostream& str) const { if (!m_dtypep) { - str << " VUP(s=" << m_stage << ",self)"; + str << " VUP(s=" << m_stage << ",self"; } else { str << " VUP(s=" << m_stage << ",dt=" << cvtToHex(dtypep()); dtypep()->dumpSmall(str); - str << ")"; } + if (m_streamUse == STREAM_USE_ASSIGN) { + str << ",stream=assign"; + } else if (m_streamUse == STREAM_USE_CAST) { + str << ",stream=cast"; + } + str << ")"; } }; std::ostream& operator<<(std::ostream& str, const WidthVP* vup) { @@ -255,6 +272,12 @@ class WidthVisitor final : public VNVisitor { EXTEND_OFF // No extension }; + static StreamUse streamUseForDeterm(Determ determ) { + return determ == ASSIGN ? STREAM_USE_ASSIGN : STREAM_USE_NONE; + } + + StreamUse currentStreamUse() const { return m_vup ? m_vup->streamUse() : STREAM_USE_NONE; } + static void packIfUnpacked(AstNodeExpr* const nodep) { if (AstUnpackArrayDType* const unpackDTypep = VN_CAST(nodep->dtypep(), UnpackArrayDType)) { const int elementsNum = unpackDTypep->arrayUnpackedElements(); @@ -970,29 +993,18 @@ class WidthVisitor final : public VNVisitor { } } bool streamImplicitUseAllowed(const AstNodeStream* nodep) const { - const AstNode* backp = nodep->backp(); - while (true) { - if (VN_IS(backp, Arg) || VN_IS(backp, SFormatArg) || VN_IS(backp, CvtArrayToArray) - || VN_IS(backp, CvtArrayToPacked) || VN_IS(backp, CvtPackedToArray) - || VN_IS(backp, CvtPackString) || VN_IS(backp, CvtUnpackedToQueue)) { - backp = backp->backp(); - } else { - break; - } - } + if (m_streamConcat) return true; + if (m_vup && m_vup->streamUseAllowed()) return true; + + const AstNode* const backp = nodep->backp(); if (!backp) return true; // Error elsewhere - // IEEE allows streaming concatenations as assignment sources/targets, nested inside - // another stream, or after an explicit cast. Other expression operands require a cast. - return VN_IS(backp, NodeAssign) // - || VN_IS(backp, Var) // - || VN_IS(backp, MemberDType) // - || VN_IS(backp, Pin) // - || VN_IS(backp, NodeFTaskRef) // - || VN_IS(backp, NodeStream) // - || VN_IS(backp, Cast) // - || VN_IS(backp, CastSize) // - || VN_IS(backp, CastWrap); + if (const AstNodeAssign* const assignp = VN_CAST(backp, NodeAssign)) { + return assignp->lhsp() == nodep || assignp->rhsp() == nodep; + } + + // Explicit cast nodes are legal contexts for the stream operand. + return VN_IS(backp, Cast) || VN_IS(backp, CastSize) || VN_IS(backp, CastWrap); } void visit(AstNodeStream* nodep) override { VL_RESTORER(m_streamConcat); @@ -2102,31 +2114,31 @@ class WidthVisitor final : public VNVisitor { void visit(AstCvtPackString* nodep) override { if (nodep->didWidthAndSet()) return; // Opaque returns, so arbitrary - userIterateAndNext(nodep->lhsp(), WidthVP{SELF, BOTH}.p()); + userIterateAndNext(nodep->lhsp(), WidthVP{SELF, BOTH, currentStreamUse()}.p()); // Type set in constructor } void visit(AstCvtPackedToArray* nodep) override { if (nodep->didWidthAndSet()) return; // Opaque returns, so arbitrary - userIterateAndNext(nodep->fromp(), WidthVP{SELF, BOTH}.p()); + userIterateAndNext(nodep->fromp(), WidthVP{SELF, BOTH, currentStreamUse()}.p()); // Type set in constructor } void visit(AstCvtArrayToArray* nodep) override { if (nodep->didWidthAndSet()) return; // Opaque returns, so arbitrary - userIterateAndNext(nodep->fromp(), WidthVP{SELF, BOTH}.p()); + userIterateAndNext(nodep->fromp(), WidthVP{SELF, BOTH, currentStreamUse()}.p()); // Type set in constructor } void visit(AstCvtArrayToPacked* nodep) override { if (nodep->didWidthAndSet()) return; // Opaque returns, so arbitrary - userIterateAndNext(nodep->fromp(), WidthVP{SELF, BOTH}.p()); + userIterateAndNext(nodep->fromp(), WidthVP{SELF, BOTH, currentStreamUse()}.p()); // Type set in constructor } void visit(AstCvtUnpackedToQueue* nodep) override { if (nodep->didWidthAndSet()) return; // Opaque returns, so arbitrary - userIterateAndNext(nodep->fromp(), WidthVP{SELF, BOTH}.p()); + userIterateAndNext(nodep->fromp(), WidthVP{SELF, BOTH, currentStreamUse()}.p()); // Type set in constructor } void visit(AstTimeImport* nodep) override { @@ -2846,7 +2858,7 @@ class WidthVisitor final : public VNVisitor { if (m_vup->final()) { UINFOTREE(9, nodep, "", "CastFPit"); iterateCheck(nodep, "value", nodep->fromp(), SELF, FINAL, nodep->fromp()->dtypep(), - EXTEND_EXP, false); + EXTEND_EXP, false, STREAM_USE_CAST); UINFOTREE(9, nodep, "", "CastFin"); AstNodeExpr* const underp = nodep->fromp()->unlinkFrBack(); underp->dtypeFrom(nodep); @@ -7544,7 +7556,7 @@ class WidthVisitor final : public VNVisitor { // Output args: at return caller = callee, reverse direction. checkClassAssign(nodep, "Function Argument", pinp, portDTypep, portp->direction() == VDirection::OUTPUT); - userIterate(pinp, WidthVP{portDTypep, FINAL}.p()); + userIterate(pinp, WidthVP{portDTypep, FINAL, STREAM_USE_ASSIGN}.p()); } else { iterateCheckAssign(nodep, "Function Argument", pinp, FINAL, portDTypep); } @@ -9213,7 +9225,7 @@ class WidthVisitor final : public VNVisitor { AstNode* iterateCheck(AstNode* parentp, const char* side, AstNode* underp, Determ determ, Stage stage, AstNodeDType* expDTypep, ExtendRule extendRule, - bool warnOn = true) { + bool warnOn = true, StreamUse streamUse = STREAM_USE_NONE) { // Perform data type check on underp, which is underneath parentp used for error reporting // Returns the new underp // Conversion to/from doubles and integers are before iterating. @@ -9221,30 +9233,37 @@ class WidthVisitor final : public VNVisitor { UASSERT_OBJ(underp, parentp, "Node has no child"); UASSERT_OBJ(underp->dtypep(), underp, "Node has no type"); // Perhaps forgot to do a prelim visit on it? + const StreamUse childStreamUse + = streamUse != STREAM_USE_NONE ? streamUse : streamUseForDeterm(determ); if (VN_IS(underp, NodeDType)) { // Note the node itself, not node's data type // Must be near top of these checks as underp->dtypep() will look normal underp->v3error(ucfirst(parentp->prettyOperatorName()) << " expected non-datatype " << side << " but " << underp->prettyNameQ() << " is a datatype."); } else if (expDTypep == underp->dtypep()) { // Perfect - underp = userIterateSubtreeReturnEdits(underp, WidthVP{expDTypep, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + underp, WidthVP{expDTypep, FINAL, childStreamUse}.p()); } else if (expDTypep->isDouble() && underp->isDouble()) { // Also good - underp = userIterateSubtreeReturnEdits(underp, WidthVP{expDTypep, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + underp, WidthVP{expDTypep, FINAL, childStreamUse}.p()); } else if (expDTypep->isDouble() && !underp->isDouble()) { AstNode* const oldp = underp; // Need FINAL on children; otherwise splice would block it spliceCvtD(VN_AS(underp, NodeExpr)); - underp = userIterateSubtreeReturnEdits(oldp, WidthVP{SELF, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + oldp, WidthVP{SELF, FINAL, childStreamUse}.p()); } else if (!expDTypep->isDouble() && underp->isDouble()) { AstNode* const oldp = underp; // Need FINAL on children; otherwise splice would block it spliceCvtS(VN_AS(underp, NodeExpr), true, expDTypep->width()); // Round RHS - underp = userIterateSubtreeReturnEdits(oldp, WidthVP{SELF, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + oldp, WidthVP{SELF, FINAL, childStreamUse}.p()); } else if (expDTypep->isString() && !underp->dtypep()->isString()) { AstNode* const oldp = underp; // Need FINAL on children; otherwise splice would block it spliceCvtString(VN_AS(underp, NodeExpr)); - underp = userIterateSubtreeReturnEdits(oldp, WidthVP{SELF, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + oldp, WidthVP{SELF, FINAL, childStreamUse}.p()); } else { const AstBasicDType* const expBasicp = expDTypep->basicp(); const AstBasicDType* const underBasicp = underp->dtypep()->basicp(); @@ -9272,7 +9291,8 @@ class WidthVisitor final : public VNVisitor { // is e.g. an ADD, the ADD will auto-adjust to the proper data type // or if another operation e.g. ATOI will not. if (determ == SELF) { - underp = userIterateSubtreeReturnEdits(underp, WidthVP{SELF, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + underp, WidthVP{SELF, FINAL, childStreamUse}.p()); } else if (determ == ASSIGN) { // IEEE: Signedness is solely determined by the RHS // (underp), not by the LHS (expDTypep) @@ -9284,9 +9304,11 @@ class WidthVisitor final : public VNVisitor { VSigning::fromBool(underp->isSigned())); UINFO(9, "Assignment of opposite-signed RHS to LHS: " << parentp); } - underp = userIterateSubtreeReturnEdits(underp, WidthVP{subDTypep, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + underp, WidthVP{subDTypep, FINAL, childStreamUse}.p()); } else { - underp = userIterateSubtreeReturnEdits(underp, WidthVP{subDTypep, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + underp, WidthVP{subDTypep, FINAL, childStreamUse}.p()); } // Note the check uses the expected size, not the child's subDTypep as we want the // child node's width to end up correct for the assignment (etc) @@ -9325,11 +9347,13 @@ class WidthVisitor final : public VNVisitor { << " interface modport on " << side << " but got " << underIfaceRefp->modportp()->prettyNameQ() << " modport."); } else { - underp = userIterateSubtreeReturnEdits(underp, WidthVP{expDTypep, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + underp, WidthVP{expDTypep, FINAL, childStreamUse}.p()); } } else { // Hope it just works out (perhaps a cast will deal with it) - underp = userIterateSubtreeReturnEdits(underp, WidthVP{expDTypep, FINAL}.p()); + underp = userIterateSubtreeReturnEdits( + underp, WidthVP{expDTypep, FINAL, childStreamUse}.p()); } } return underp; diff --git a/test_regress/t/t_display_stream_bad.out b/test_regress/t/t_display_stream_bad.out index f667f0005..bf7c19963 100644 --- a/test_regress/t/t_display_stream_bad.out +++ b/test_regress/t/t_display_stream_bad.out @@ -39,4 +39,9 @@ : ... Suggest use a cast 26 | void'($sformatf("%0d", {<<{value}})); | ^~ +%Error: t/t_display_stream_bad.v:27:27: Streaming concatenation cannot be used in an implicitly cast context (IEEE 1800-2023 11.4.17) + : ... note: In instance 't' + : ... Suggest use a cast + 27 | result = passthrough({<<{value}} + 1); + | ^~ %Error: Exiting due to diff --git a/test_regress/t/t_display_stream_bad.v b/test_regress/t/t_display_stream_bad.v index 6df222cf7..6fe7bd0db 100644 --- a/test_regress/t/t_display_stream_bad.v +++ b/test_regress/t/t_display_stream_bad.v @@ -24,6 +24,8 @@ module t; $display({<<{value}}); $display("%0d", {<<{value}}); void'($sformatf("%0d", {<<{value}})); + result = passthrough({<<{value}} + 1); result = int'({<<{value}}); + result = int'({<<{value}}) + 1; end endmodule