From cedec65c39f20e12bf67ea71c4b95f578e566ba7 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Fri, 6 Mar 2026 09:32:08 +0000 Subject: [PATCH] Change array tracing to always dump left index to right index (#7205) Also add array bounds and struct/union member counts to trace pushPrefix (not used by vcd/fst/saif). Together these improve consistency in some waveform formats. --- include/verilated_fst_c.h | 6 ++++-- include/verilated_saif_c.h | 6 ++++-- include/verilated_vcd_c.h | 5 +++-- src/V3AstNodeStmt.h | 11 +++++++++-- src/V3EmitCImp.cpp | 16 +++++++++++++--- src/V3TraceDecl.cpp | 31 +++++++++++++++++++++++-------- test_regress/driver.py | 4 ++-- 7 files changed, 58 insertions(+), 21 deletions(-) diff --git a/include/verilated_fst_c.h b/include/verilated_fst_c.h index c4c4288d8..bc243b502 100644 --- a/include/verilated_fst_c.h +++ b/include/verilated_fst_c.h @@ -148,8 +148,10 @@ public: const char** itemNamesp, const char** itemValuesp); }; -// duck-typed interface to decl* methods -// We use macros in order to strip out unused args at compile time. +// We use macros to drop unused arguments at compile time. This saves code size. +#define VL_TRACE_PUSH_PREFIX(tracep, name, type, left, right) tracep->pushPrefix(name, type); +#define VL_TRACE_POP_PREFIX(tracep) tracep->popPrefix(); + #define VL_TRACE_DECL_EVENT(tracep, code, fidx, name, dtypenum, dir, kind, type) \ tracep->declEvent(code, name, dtypenum, dir, kind, type) #define VL_TRACE_DECL_BIT(tracep, code, fidx, name, dtypenum, dir, kind, type) \ diff --git a/include/verilated_saif_c.h b/include/verilated_saif_c.h index f9ba20731..b3a46fa6c 100644 --- a/include/verilated_saif_c.h +++ b/include/verilated_saif_c.h @@ -166,8 +166,10 @@ public: void declDoubleArray(uint32_t code, uint32_t fidx, const char* name, int arraynum); }; -// duck-typed interface to decl* methods -// We use macros in order to strip out unused args at compile time. +// We use macros to drop unused arguments at compile time. This saves code size. +#define VL_TRACE_PUSH_PREFIX(tracep, name, type, left, right) tracep->pushPrefix(name, type); +#define VL_TRACE_POP_PREFIX(tracep) tracep->popPrefix(); + #define VL_TRACE_DECL_EVENT(tracep, code, fidx, name, dtypenum, dir, kind, type) \ tracep->declEvent(code, fidx, name) #define VL_TRACE_DECL_BIT(tracep, code, fidx, name, dtypenum, dir, kind, type) \ diff --git a/include/verilated_vcd_c.h b/include/verilated_vcd_c.h index 829bbc70b..4731facfe 100644 --- a/include/verilated_vcd_c.h +++ b/include/verilated_vcd_c.h @@ -154,8 +154,9 @@ public: void declDoubleArray(uint32_t code, const char* name, int arraynum); }; -// duck-typed interface to decl* methods -// We use macros in order to strip out unused args at compile time. +// We use macros to drop unused arguments at compile time. This saves code size. +#define VL_TRACE_PUSH_PREFIX(tracep, name, type, left, right) tracep->pushPrefix(name, type); +#define VL_TRACE_POP_PREFIX(tracep) tracep->popPrefix(); #define VL_TRACE_DECL_EVENT(tracep, code, fidx, name, dtypenum, dir, kind, type) \ tracep->declEvent(code, name) diff --git a/src/V3AstNodeStmt.h b/src/V3AstNodeStmt.h index 2430b1641..a5662f632 100644 --- a/src/V3AstNodeStmt.h +++ b/src/V3AstNodeStmt.h @@ -1345,15 +1345,22 @@ public: class AstTracePushPrefix final : public AstNodeStmt { const string m_prefix; // Prefix to add to signal names const VTracePrefixType m_prefixType; // Type of prefix being pushed + const int m_left; // Array left index, or struct/union member count + const int m_right; // Array right index public: - AstTracePushPrefix(FileLine* fl, const string& prefix, VTracePrefixType prefixType) + AstTracePushPrefix(FileLine* fl, const string& prefix, VTracePrefixType prefixType, + int left = 0, int right = 0) : ASTGEN_SUPER_TracePushPrefix(fl) , m_prefix{prefix} - , m_prefixType{prefixType} {} + , m_prefixType{prefixType} + , m_left{left} + , m_right{right} {} ASTGEN_MEMBERS_AstTracePushPrefix; bool sameNode(const AstNode* samep) const override { return false; } string prefix() const { return m_prefix; } VTracePrefixType prefixType() const { return m_prefixType; } + int left() const { return m_left; } + int right() const { return m_right; } }; class AstWait final : public AstNodeStmt { // @astgen op1 := condp : AstNodeExpr diff --git a/src/V3EmitCImp.cpp b/src/V3EmitCImp.cpp index d58fb3393..e093fc04e 100644 --- a/src/V3EmitCImp.cpp +++ b/src/V3EmitCImp.cpp @@ -682,7 +682,11 @@ class EmitCTrace final : public EmitCFunc { // Array range if (nodep->arrayRange().ranged()) { - puts(", (i+" + cvtToStr(nodep->arrayRange().lo()) + ")"); + if (nodep->arrayRange().ascending()) { + puts(", (i + " + std::to_string(nodep->arrayRange().lo()) + ")"); + } else { + puts(", (" + std::to_string(nodep->arrayRange().hi()) + " - i)"); + } } // Bit range @@ -742,6 +746,10 @@ class EmitCTrace final : public EmitCFunc { : "(oldp+"); puts(cvtToStr(code - nodep->baseCode())); puts(","); + const VNumRange& arrayRange = nodep->declp()->arrayRange(); + if (arrayRange.ranged() && !arrayRange.ascending()) { + arrayindex = arrayRange.elements() - 1 - arrayindex; + } emitTraceValue(nodep, arrayindex); if (emitWidth) puts("," + cvtToStr(nodep->declp()->widthMin())); puts(");\n"); @@ -802,14 +810,16 @@ class EmitCTrace final : public EmitCFunc { EmitCFunc::visit(nodep); } void visit(AstTracePushPrefix* nodep) override { - putns(nodep, "tracep->pushPrefix("); + putns(nodep, "VL_TRACE_PUSH_PREFIX(tracep, "); putsQuoted(VIdProtect::protectWordsIf(nodep->prefix(), nodep->protect())); puts(", VerilatedTracePrefixType::"); puts(nodep->prefixType().ascii()); + puts(", " + std::to_string(nodep->left())); + puts(", " + std::to_string(nodep->right())); puts(");\n"); } void visit(AstTracePopPrefix* nodep) override { // - putns(nodep, "tracep->popPrefix();\n"); + putns(nodep, "VL_TRACE_POP_PREFIX(tracep);\n"); } void visit(AstTraceDecl* nodep) override { const int enumNum = emitTraceDeclDType(nodep->dtypep()); diff --git a/src/V3TraceDecl.cpp b/src/V3TraceDecl.cpp index 82d6dae2a..c76e3617c 100644 --- a/src/V3TraceDecl.cpp +++ b/src/V3TraceDecl.cpp @@ -574,7 +574,8 @@ class TraceDeclVisitor final : public VNVisitor { VL_RESTORER(m_traName); FileLine* const flp = nodep->fileline(); - addToSubFunc(new AstTracePushPrefix{flp, m_traName, VTracePrefixType::ARRAY_UNPACKED}); + addToSubFunc(new AstTracePushPrefix{flp, m_traName, VTracePrefixType::ARRAY_UNPACKED, + nodep->left(), nodep->right()}); if (VN_IS(nodep->subDTypep()->skipRefToEnump(), BasicDType) // Nothing lower than this array @@ -589,7 +590,9 @@ class TraceDeclVisitor final : public VNVisitor { } } else { AstNodeDType* const subtypep = nodep->subDTypep()->skipRefToEnump(); - for (int i = nodep->lo(); i <= nodep->hi(); ++i) { + // Always iterate left index to right index + const int inc = nodep->rangep()->ascending() ? 1 : -1; + for (int i = nodep->left(); i != nodep->right() + inc; i += inc) { VL_RESTORER(m_traValuep); m_traName = '[' + std::to_string(i) + ']'; m_traValuep = m_traValuep->cloneTree(false); @@ -615,10 +618,14 @@ class TraceDeclVisitor final : public VNVisitor { VL_RESTORER(m_traName); FileLine* const flp = nodep->fileline(); - addToSubFunc(new AstTracePushPrefix{flp, m_traName, VTracePrefixType::ARRAY_PACKED}); + + addToSubFunc(new AstTracePushPrefix{flp, m_traName, VTracePrefixType::ARRAY_PACKED, + nodep->left(), nodep->right()}); AstNodeDType* const subtypep = nodep->subDTypep()->skipRefToEnump(); - for (int i = nodep->lo(); i <= nodep->hi(); ++i) { + // Always iterate left index to right index + const int inc = nodep->rangep()->ascending() ? 1 : -1; + for (int i = nodep->left(); i != nodep->right() + inc; i += inc) { VL_RESTORER(m_traValuep); m_traName = '[' + std::to_string(i) + ']'; const int lsb = (i - nodep->lo()) * subtypep->width(); @@ -645,9 +652,12 @@ class TraceDeclVisitor final : public VNVisitor { VL_RESTORER(m_traName); FileLine* const flp = nodep->fileline(); + int nMembers = 0; + for (AstNode* mp = nodep->membersp(); mp; mp = mp->nextp()) ++nMembers; + if (!nodep->packed()) { - addToSubFunc( - new AstTracePushPrefix{flp, m_traName, VTracePrefixType::STRUCT_UNPACKED}); + addToSubFunc(new AstTracePushPrefix{flp, m_traName, // + VTracePrefixType::STRUCT_UNPACKED, nMembers}); for (const AstMemberDType *itemp = nodep->membersp(), *nextp; itemp; itemp = nextp) { nextp = VN_AS(itemp->nextp(), MemberDType); AstNodeDType* const subtypep = itemp->subDTypep()->skipRefToEnump(); @@ -661,7 +671,8 @@ class TraceDeclVisitor final : public VNVisitor { } addToSubFunc(new AstTracePopPrefix{flp}); } else { - addToSubFunc(new AstTracePushPrefix{flp, m_traName, VTracePrefixType::STRUCT_PACKED}); + addToSubFunc(new AstTracePushPrefix{flp, m_traName, // + VTracePrefixType::STRUCT_PACKED, nMembers}); for (const AstMemberDType *itemp = nodep->membersp(), *nextp; itemp; itemp = nextp) { nextp = VN_AS(itemp->nextp(), MemberDType); AstNodeDType* const subtypep = itemp->subDTypep()->skipRefToEnump(); @@ -690,10 +701,14 @@ class TraceDeclVisitor final : public VNVisitor { VL_RESTORER(m_traName); FileLine* const flp = nodep->fileline(); + int nMembers = 0; + for (AstNode* mp = nodep->membersp(); mp; mp = mp->nextp()) ++nMembers; + if (!nodep->packed()) { addIgnore("Unsupported: Unpacked union"); } else { - addToSubFunc(new AstTracePushPrefix{flp, m_traName, VTracePrefixType::UNION_PACKED}); + addToSubFunc(new AstTracePushPrefix{flp, m_traName, // + VTracePrefixType::UNION_PACKED, nMembers}); for (const AstMemberDType *itemp = nodep->membersp(), *nextp; itemp; itemp = nextp) { nextp = VN_AS(itemp->nextp(), MemberDType); AstNodeDType* const subtypep = itemp->subDTypep()->skipRefToEnump(); diff --git a/test_regress/driver.py b/test_regress/driver.py index acec5a6c1..75ee49219 100755 --- a/test_regress/driver.py +++ b/test_regress/driver.py @@ -2569,7 +2569,7 @@ class VlTest: var = [] for line in fh: match1 = re.search(r'\$scope (module|struct|interface)\s+(\S+)', line) - match2 = re.search(r'(\$var (\S+)\s+\d+\s+)\S+\s+(\S+)', line) + match2 = re.search(r'(\$var \S+\s+\d+\s+)\S+\s+(.+)\s+\$end', line) match3 = re.search(r'(\$attrbegin .* \$end)', line) line = line.rstrip() # print("VR"+ ' '*len(hier_stack) +" L " + line) @@ -2583,7 +2583,7 @@ class VlTest: # print("VR"+ ' '*len(hier_stack) +" var " + line) scope = '.'.join(hier_stack) var = match2.group(2) - data[scope + "." + var] = match2.group(1) + match2.group(3) + data[scope + "." + var] = match2.group(1) elif match3: # $attrbegin # print("VR"+ ' '*len(hier_stack) +" attr " + line) if var: