From 929e15fa4c61751a247ae6b3cc17bcf32bddb86b Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Fri, 24 Jan 2025 21:00:45 -0500 Subject: [PATCH] Fix various round-trip Verilog output, including packed arrays --- src/V3EmitV.cpp | 190 +++++++++++++++++++++---------- test_regress/t/t_debug_emitv.out | 166 +++++++++++---------------- test_regress/t/t_debug_emitv.py | 13 ++- test_regress/t/t_debug_emitv.v | 36 ++++-- 4 files changed, 233 insertions(+), 172 deletions(-) diff --git a/src/V3EmitV.cpp b/src/V3EmitV.cpp index c61d3c0cc..8aab51688 100644 --- a/src/V3EmitV.cpp +++ b/src/V3EmitV.cpp @@ -28,13 +28,17 @@ VL_DEFINE_DEBUG_FUNCTIONS; // Emit statements and expressions class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { - // MEMBERS - bool m_suppressSemi = false; + // STATE - across all visitors const bool m_suppressUnknown = false; + + // STATE - for current visit position (use VL_RESTORER) AstSenTree* m_sensesp; // Domain for printing one a ALWAYS under a ACTIVE + bool m_suppressSemi = false; // Non-statement, don't print ; + bool m_suppressVarSemi = false; // Suppress emitting semicolon for AstVars + bool m_arrayPost = false; // Print array information that goes after identifier (vs after) + std::deque m_packedps; // Packed arrays to print with BasicDType // METHODS - virtual void puts(const string& str) = 0; virtual void putbs(const string& str) = 0; virtual void putfs(AstNode* nodep, const string& str) = 0; // Fileline and node %% mark @@ -49,6 +53,20 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { putsNoTracking("\""); } + void iterateAndCommaConstNull(AstNode* nodep) { + for (; nodep; nodep = nodep->nextp()) { + iterateConst(nodep); + if (nodep->nextp()) puts(", "); + } + } + void emitPacked() { + for (AstNodeArrayDType* packedp : m_packedps) { + puts(" "); + iterateConstNull(packedp->rangep()); + } + m_packedps.clear(); + } + // VISITORS void visit(AstNetlist* nodep) override { iterateAndNextConstNull(nodep->modulesp()); } void visit(AstNodeModule* nodep) override { @@ -58,13 +76,14 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { } void visit(AstPort* nodep) override {} void visit(AstNodeFTask* nodep) override { - putfs(nodep, nodep->isFunction() ? "function" : "task"); + const bool func = nodep->isFunction() || nodep->name() == "new"; + putfs(nodep, func ? "function" : "task"); puts(" "); puts(nodep->prettyName()); puts(";\n"); // Only putfs the first time for each visitor; later for same node is putqs iterateAndNextConstNull(nodep->stmtsp()); - putfs(nodep, nodep->isFunction() ? "endfunction\n" : "endtask\n"); + putfs(nodep, func ? "endfunction\n" : "endtask\n"); } void visit(AstBegin* nodep) override { @@ -169,7 +188,7 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { } void visit(AstSenItem* nodep) override { putfs(nodep, ""); - puts(nodep->edgeType().verilogKwd()); + if (nodep->edgeType() != VEdgeType::ET_CHANGED) puts(nodep->edgeType().verilogKwd()); if (nodep->sensp()) puts(" "); iterateChildrenConst(nodep); } @@ -221,13 +240,13 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { putfs(nodep, nodep->verilogKwd()); putbs("("); if (fileOrStrgp) { - iterateAndNextConstNull(fileOrStrgp); + iterateConstNull(fileOrStrgp); putbs(", "); } putsQuoted(text); for (AstNode* expp = exprsp; expp; expp = expp->nextp()) { puts(", "); - iterateAndNextConstNull(expp); + iterateConstNull(expp); } puts(");\n"); } @@ -393,7 +412,7 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { } void visit(AstTextBlock* nodep) override { visit(static_cast(nodep)); - VL_RESTORER(m_suppressSemi); + VL_RESTORER(m_suppressVarSemi); m_suppressVarSemi = nodep->commas(); for (AstNode* childp = nodep->nodesp(); childp; childp = childp->nextp()) { iterateConst(childp); @@ -403,17 +422,17 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { void visit(AstScopeName* nodep) override {} void visit(AstCStmt* nodep) override { putfs(nodep, "$_CSTMT("); - iterateAndNextConstNull(nodep->exprsp()); + iterateAndCommaConstNull(nodep->exprsp()); puts(");\n"); } void visit(AstCExpr* nodep) override { putfs(nodep, "$_CEXPR("); - iterateAndNextConstNull(nodep->exprsp()); + iterateAndCommaConstNull(nodep->exprsp()); puts(")"); } void visit(AstUCStmt* nodep) override { putfs(nodep, "$c("); - iterateAndNextConstNull(nodep->exprsp()); + iterateAndCommaConstNull(nodep->exprsp()); puts(");\n"); } void visit(AstUCFunc* nodep) override { @@ -431,19 +450,13 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { void visit(AstCMethodHard* nodep) override { iterateConst(nodep->fromp()); puts("." + nodep->name() + "("); - for (AstNode* pinp = nodep->pinsp(); pinp; pinp = pinp->nextp()) { - if (pinp != nodep->pinsp()) puts(", "); - iterateConst(pinp); - } + iterateAndCommaConstNull(nodep->pinsp()); puts(")"); } void visit(AstCMethodCall* nodep) override { iterateConst(nodep->fromp()); puts("." + nodep->name() + "("); - for (AstNode* pinp = nodep->argsp(); pinp; pinp = pinp->nextp()) { - if (pinp != nodep->argsp()) puts(", "); - iterateConst(pinp); - } + iterateAndCommaConstNull(nodep->argsp()); puts(")"); } @@ -565,13 +578,15 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { puts(cvtToStr(nodep->leftConst())); puts(":"); puts(cvtToStr(nodep->rightConst())); - puts("]"); } else { iterateAndNextConstNull(nodep->leftp()); puts(":"); iterateAndNextConstNull(nodep->rightp()); - puts("]"); } + puts("]"); + } + void visit(AstRand* nodep) override { + emitVerilogFormat(nodep, nodep->emitVerilog(), nodep->seedp()); } void visit(AstSel* nodep) override { iterateAndNextConstNull(nodep->fromp()); @@ -603,17 +618,31 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { } void visit(AstTypedef* nodep) override { putfs(nodep, "typedef "); - iterateAndNextConstNull(nodep->subDTypep()); + iterateConstNull(nodep->subDTypep()); puts(" "); puts(nodep->prettyName()); puts(";\n"); } + void visit(AstAssocArrayDType* nodep) override { + if (!m_arrayPost) { + iterateConst(nodep->subDTypep()); + } else { + VL_RESTORER(m_arrayPost); + m_arrayPost = false; + puts("["); + iterateConst(nodep->keyDTypep()); + puts("]"); + m_arrayPost = true; + iterateConst(nodep->subDTypep()); // For post's key + } + } void visit(AstBasicDType* nodep) override { + if (m_arrayPost) return; putfs(nodep, nodep->prettyName()); - if (nodep->isSigned()) putfs(nodep, " signed"); + if (nodep->isSigned() && !nodep->keyword().isDouble()) putfs(nodep, " signed"); // Do not emit ranges for integer atoms. if (nodep->keyword().isIntNumeric() && !nodep->keyword().isBitLogic()) return; - + emitPacked(); if (nodep->rangep()) { puts(" "); iterateAndNextConstNull(nodep->rangep()); @@ -627,12 +656,50 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { } } void visit(AstConstDType* nodep) override { + if (m_arrayPost) return; putfs(nodep, "const "); iterateConst(nodep->subDTypep()); } - void visit(AstNodeArrayDType* nodep) override { + void visit(AstDynArrayDType* nodep) override { + if (!m_arrayPost) { + iterateConst(nodep->subDTypep()); + } else { + puts("[]"); + iterateConst(nodep->subDTypep()); // For post's key + } + } + void visit(AstEnumDType* nodep) override { + if (m_arrayPost) return; + putfs(nodep, "enum "); iterateConst(nodep->subDTypep()); - iterateAndNextConstNull(nodep->rangep()); + puts("{\n"); + iterateAndNextConstNull(nodep->itemsp()); + puts("}"); + } + void visit(AstEnumItem* nodep) override { + putfs(nodep, nodep->name()); + iterateConstNull(nodep->rangep()); + puts(" = "); + iterateConstNull(nodep->valuep()); + if (nodep->nextp()) puts(","); + puts("\n"); + } + void visit(AstNodeArrayDType* nodep) override { + if (!m_arrayPost) { + if (VN_IS(nodep, PackArrayDType)) { + // Unpacked ranges handled in BasicDType, as they print "backwards" + m_packedps.push_back(nodep); + } + iterateConst(nodep->subDTypep()); + } else { + if (VN_IS(nodep, UnpackArrayDType)) { + VL_RESTORER(m_arrayPost); + m_arrayPost = false; + iterateAndNextConstNull(nodep->rangep()); + m_arrayPost = true; + } + iterateConst(nodep->subDTypep()); // For post's key + } } void visit(AstRefDType* nodep) override { if (nodep->subDTypep()) { @@ -642,21 +709,43 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { } } void visit(AstNodeUOrStructDType* nodep) override { + if (m_arrayPost) return; puts(nodep->verilogKwd() + " "); if (nodep->packed()) puts("packed "); - puts("\n"); - puts("{"); - for (AstMemberDType* itemp = nodep->membersp(); itemp; - itemp = VN_AS(itemp->nextp(), MemberDType)) { - iterateConst(itemp); - puts(";"); + { + puts("{\n"); + VL_RESTORER(m_packedps); + m_packedps.clear(); + for (AstMemberDType* itemp = nodep->membersp(); itemp; + itemp = VN_AS(itemp->nextp(), MemberDType)) { + iterateConst(itemp); + } + puts("}"); } - puts("}"); + emitPacked(); } void visit(AstMemberDType* nodep) override { + if (m_arrayPost) return; iterateConst(nodep->subDTypep()); puts(" "); puts(nodep->name()); + puts(";\n"); + } + void visit(AstQueueDType* nodep) override { + if (!m_arrayPost) { + iterateConst(nodep->subDTypep()); + } else { + VL_RESTORER(m_arrayPost); + m_arrayPost = false; + puts("[$"); + if (nodep->boundp()) { + puts(":"); + iterateConst(nodep->boundp()); + } + puts("]"); + m_arrayPost = true; + iterateConst(nodep->subDTypep()); // For post's key + } } void visit(AstNodeFTaskRef* nodep) override { if (nodep->dotted() != "") { @@ -719,37 +808,23 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { putfs(nodep, nodep->verilogKwd()); puts(" "); } - std::vector unpackps; - for (AstNodeDType* dtypep = nodep->dtypep(); dtypep;) { - dtypep = dtypep->skipRefp(); - if (const AstUnpackArrayDType* const unpackp = VN_CAST(dtypep, UnpackArrayDType)) { - unpackps.push_back(unpackp); - dtypep = unpackp->subDTypep(); - } else { - iterateConst(dtypep); - puts(" "); - puts(nodep->prettyName()); - dtypep = nullptr; - } - } - // If nodep is an unpacked array, append unpacked dimensions - for (const auto& unpackp : unpackps) { - puts("["); - puts(cvtToStr(unpackp->rangep()->leftConst())); - puts(":"); - puts(cvtToStr(unpackp->rangep()->rightConst())); - puts("]"); - } + VL_RESTORER(m_arrayPost); + m_arrayPost = false; + iterateConstNull(nodep->dtypep()); // Dtype part before identifier + puts(" "); + puts(nodep->prettyName()); + m_arrayPost = true; + iterateConstNull(nodep->dtypep()); // Dtype part after identifier puts(m_suppressVarSemi ? "\n" : ";\n"); } void visit(AstActive* nodep) override { + VL_RESTORER(m_sensesp); m_sensesp = nodep->sensesp(); iterateAndNextConstNull(nodep->stmtsp()); - m_sensesp = nullptr; } void visit(AstParseRef* nodep) override { puts(nodep->prettyName()); } - void visit(AstVarScope*) override {} void visit(AstNodeText*) override {} + void visit(AstVarScope*) override {} void visit(AstTraceDecl*) override {} void visit(AstTraceInc*) override {} // NOPs @@ -767,7 +842,6 @@ class EmitVBaseVisitorConst VL_NOT_FINAL : public EmitCBaseVisitorConst { } public: - bool m_suppressVarSemi = false; // Suppress emitting semicolon for AstVars explicit EmitVBaseVisitorConst(bool suppressUnknown, AstSenTree* domainp) : m_suppressUnknown{suppressUnknown} , m_sensesp{domainp} {} diff --git a/test_regress/t/t_debug_emitv.out b/test_regress/t/t_debug_emitv.out index 4e2b89ac7..d7337c931 100644 --- a/test_regress/t/t_debug_emitv.out +++ b/test_regress/t/t_debug_emitv.out @@ -1,94 +1,65 @@ module Vt_debug_emitv_t; input logic clk; input logic in; - typedef - ???? // ENUMDTYPE 't.e_t' - - ???? // ENUMITEM 'ZERO' - 32'h0 - ???? // ENUMITEM 'ONE' - 32'h1int signedstruct packed - {int signed a;}logic signed [2:0] struct - {logic signed [2:0] a;}logicunion - {logic a;}struct packed - {int signed a;}bit [31:0] const struct packed - {int signed a;}const struct packed - {int signed a;}[0:2]struct - {logic signed [2:0] a;}union - {logic a;}int signedint signed[0:2]logic [15:0] logic [15:0] logic [15:0] int signedint signedint signed - ???? // QUEUEDTYPE - int signedstring - ???? // ASSOCARRAYDTYPE - int signed - ???? // DYNARRAYDTYPE - int signedint signedint signedint signedint signedint signedint signedreal signedstringIDatalogic signed [31:0] int signed e_t; - typedef struct packed - {int signed a;}logic signed [2:0] struct - {logic signed [2:0] a;}logicunion - {logic a;}struct packed - {int signed a;}bit [31:0] const struct packed - {int signed a;}const struct packed - {int signed a;}[0:2]struct - {logic signed [2:0] a;}union - {logic a;}int signedint signed[0:2]logic [15:0] logic [15:0] logic [15:0] int signedint signedint signed - ???? // QUEUEDTYPE - int signedstring - ???? // ASSOCARRAYDTYPE - int signed - ???? // DYNARRAYDTYPE - int signedint signedint signedint signedint signedint signedint signedreal signedstringIDatalogic signed [31:0] int signed ps_t; - typedef struct - {logic signed [2:0] a;}logicunion - {logic a;}struct packed - {int signed a;}bit [31:0] const struct packed - {int signed a;}const struct packed - {int signed a;}[0:2]struct - {logic signed [2:0] a;}union - {logic a;}int signedint signed[0:2]logic [15:0] logic [15:0] logic [15:0] int signedint signedint signed - ???? // QUEUEDTYPE - int signedstring - ???? // ASSOCARRAYDTYPE - int signed - ???? // DYNARRAYDTYPE - int signedint signedint signedint signedint signedint signedint signedreal signedstringIDatalogic signed [31:0] int signed us_t; - typedef union - {logic a;}struct packed - {int signed a;}bit [31:0] const struct packed - {int signed a;}const struct packed - {int signed a;}[0:2]struct - {logic signed [2:0] a;}union - {logic a;}int signedint signed[0:2]logic [15:0] logic [15:0] logic [15:0] int signedint signedint signed - ???? // QUEUEDTYPE - int signedstring - ???? // ASSOCARRAYDTYPE - int signed - ???? // DYNARRAYDTYPE - int signedint signedint signedint signedint signedint signedint signedreal signedstringIDatalogic signed [31:0] int signed union_t; - struct packed - {int signed a;} ps[0:2]; - struct - {logic signed [2:0] a;} us; - union - {logic a;} unu; + typedef enum logic [2:0] { + ZERO = 3'h0, + ONE = 3'h1 + } e_t; + typedef struct packed { + logic [2:0] a; + } ps_t; + typedef struct { + logic signed [2:0] a; + } us_t; + typedef union { + logic a; + } union_t; + const struct packed { + logic [2:0] a; + } ps[0:2]; + struct { + logic signed [2:0] a; + } us; + union { + logic a; + } unu; int signed array[0:2]; initial begin array = '{0:32'sh1, 1:32'sh2, 2:32'sh3}; end + bit [6:5] [4:3] [2:1] arraymanyd[10:11][12:13][14:15]; logic [15:0] pubflat; logic [15:0] pubflat_r; logic [15:0] pubflat_w; assign pubflat_w = pubflat; int signed fd; int signed i; - - ???? // QUEUEDTYPE - q; - - ???? // ASSOCARRAYDTYPE - assoc; - - ???? // DYNARRAYDTYPE - dyn; + int signed q[$]; + int signed qb[$:'sh3]; + int signed assoc[string]; + int signed assocassoc[string][real]; + int signed dyn[]; + typedef struct packed { + logic nn1; + } nested_named_t; + typedef struct packed { + struct packed { + logic nn2; + } nested_anonymous; + struct packed { + logic nn1; + } nested_named; + logic [11:10] nn3; + } nibble_t; + struct packed { + struct packed { + logic nn2; + } nested_anonymous; + struct packed { + logic nn1; + } nested_named; + logic [11:10] nn3; + } [5:4] nibblearray[3:2]; task t; $display("stmt"); endtask @@ -111,7 +82,7 @@ module Vt_debug_emitv_t; begin other = f(i); $display("stmt %~ %~", - iother, other); + i, other); t(); end i = (i + 'h1); @@ -128,7 +99,7 @@ module Vt_debug_emitv_t; $display("stmt"); end end - always @([changed] in) begin + always @( in) begin begin $display("stmt"); end @@ -147,7 +118,7 @@ module Vt_debug_emitv_t; int signed cyc; int signed fo; int signed sum; - real signed r; + real r; string str; always @(posedge clk) begin begin @@ -156,7 +127,7 @@ module Vt_debug_emitv_t; fo = cyc; sub.inc(fosum); sum = sub.f(sum); - $display("[%0t] sum = %~", $timesum, sum); + $display("[%0t] sum = %~", $time, sum); $display("a?= %d", ($c('sh1) ? $c('sh14) : $c('sh1e))); $c(;); @@ -248,32 +219,23 @@ module Vt_debug_emitv_t; else begin $display("0"); end - $display("%~%~", $past(cyc)$past(cyc, 'sh1), - $past(cyc, 'sh1)); + $display("%~%~", $past(cyc), $past(cyc, + 'sh1)); str = $sformatf("cyc=%~", cyc); ; $display("str = %@", str); - $display("%% [%t] [%^] to=%o td=%d", $time - $realtime$time$time, $realtime - $time$time, $time$time, $time); + $display("%% [%t] [%^] to=%o td=%d", $time, + $realtime, $time, $time); $sscanf(40'h666f6f3d35, "foo=%d", i); ; $printtimescale; if ((i != 'sh5)) begin $stop; end - sum = - ???? // RAND - ; - sum = - ???? // RAND - 'sha; - sum = - ???? // RAND - ; - sum = - ???? // RAND - 'sha; + sum = $random(); + sum = $random('sha); + sum = $urandom(); + sum = $urandom('sha); if ((PKG_PARAM != 'sh1)) begin $stop; end @@ -318,8 +280,8 @@ package Vt_debug_emitv___024unit; member = 'sh1; task method; endtask - task new; - endtask + function new; + endfunction endclass endpackage module Vt_debug_emitv_sub; @@ -339,7 +301,7 @@ module Vt_debug_emitv_sub; disable label0; end endfunction - real signed r; + real r; endmodule package Vt_debug_emitv_p; logic pkgvar; diff --git a/test_regress/t/t_debug_emitv.py b/test_regress/t/t_debug_emitv.py index 6063ac2c8..227968047 100755 --- a/test_regress/t/t_debug_emitv.py +++ b/test_regress/t/t_debug_emitv.py @@ -16,7 +16,16 @@ test.lint( # Likewise XML v_flags=["--lint-only --dumpi-tree 9 --dumpi-V3EmitV 9 --debug-emitv"]) -test.files_identical(test.glob_one(test.obj_dir + "/" + test.vm_prefix + "_*_width.tree.v"), - test.golden_filename) +output_v = test.glob_one(test.obj_dir + "/" + test.vm_prefix + "_*_width.tree.v") + +test.files_identical(output_v, test.golden_filename) + +if test.verbose: + # Print if that the output Verilog is clean + # TODO not yet round-trip clean + test.run(cmd=[os.environ["VERILATOR_ROOT"] + "/bin/verilator", "--lint-only", output_v], + logfile=test.obj_dir + "/sim_roundtrip.log", + fails=True, + verilator_run=True) test.passes() diff --git a/test_regress/t/t_debug_emitv.v b/test_regress/t/t_debug_emitv.v index 989888d8c..8c7ba061a 100644 --- a/test_regress/t/t_debug_emitv.v +++ b/test_regress/t/t_debug_emitv.v @@ -33,7 +33,7 @@ module t (/*AUTOARG*/ // verilator lint_off UNPACKED - typedef enum { + typedef enum [2:0] { ZERO, ONE = 1 } e_t; @@ -52,19 +52,35 @@ module t (/*AUTOARG*/ us_t us; union_t unu; - int array[3]; + int array[3]; initial array = '{1,2,3}; - reg [15:0] pubflat /*verilator public_flat_rw @(posedge clk) */; + bit [6:5][4:3][2:1] arraymanyd[10:11][12:13][14:15]; - reg [15:0] pubflat_r; - wire [15:0] pubflat_w = pubflat; - int fd; - int i; + reg [15:0] pubflat /*verilator public_flat_rw @(posedge clk) */; - int q[$]; - int assoc[string]; - int dyn[]; + reg [15:0] pubflat_r; + wire [15:0] pubflat_w = pubflat; + int fd; + int i; + + int q[$]; + int qb[$ : 3]; + int assoc[string]; + int assocassoc[string][real]; + int dyn[]; + + typedef struct packed { + logic nn1; + } nested_named_t; + typedef struct packed { + struct packed { + logic nn2; + } nested_anonymous; + nested_named_t nested_named; + logic [11:10] nn3; + } nibble_t; + nibble_t [5:4] nibblearray[3:2]; task t; $display("stmt");