diff --git a/src/V3Ast.h b/src/V3Ast.h index c527c71f9..dbc31cdab 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1509,6 +1509,16 @@ public: AstNode* firstAbovep() const { // Returns nullptr when second or later in list return ((backp() && backp()->nextp() != this) ? backp() : nullptr); } + // isFirstInMyListOfStatements(n) -- implemented by child classes: + // AstNodeBlock, AstCaseItem, AstNodeIf, AstNodeFTask, and possibly others. + virtual bool isFirstInMyListOfStatements(AstNode* n) const { return false; } + // isStandaloneBodyStmt == Do we need a ; on generated cpp for this node? + bool isStandaloneBodyStmt() { + return (!firstAbovep() // we're 2nd or later in the list, so yes need ; + + // If we're first in the list, check what backp() thinks of us: + || (backp() && backp()->isFirstInMyListOfStatements(this))); + } uint8_t brokenState() const { return m_brokenState; } void brokenState(uint8_t value) { m_brokenState = value; } @@ -2566,6 +2576,7 @@ public: AstNode* stmtsp() const { return op1p(); } // op1 = List of statements void addStmtsp(AstNode* nodep) { addNOp1p(nodep); } bool unnamed() const { return m_unnamed; } + bool isFirstInMyListOfStatements(AstNode* nodep) const override { return nodep == stmtsp(); } }; class AstNodePreSel VL_NOT_FINAL : public AstNode { @@ -2711,6 +2722,9 @@ public: VBranchPred branchPred() const { return m_branchPred; } void isBoundsCheck(bool flag) { m_isBoundsCheck = flag; } bool isBoundsCheck() const { return m_isBoundsCheck; } + bool isFirstInMyListOfStatements(AstNode* n) const override { + return n == ifsp() || n == elsesp(); + } }; class AstNodeCase VL_NOT_FINAL : public AstNodeStmt { @@ -3237,6 +3251,7 @@ public: bool isVirtual() const { return m_virtual; } void lifetime(const VLifetime& flag) { m_lifetime = flag; } VLifetime lifetime() const { return m_lifetime; } + bool isFirstInMyListOfStatements(AstNode* n) const override { return n == stmtsp(); } }; class AstNodeFTaskRef VL_NOT_FINAL : public AstNodeStmt { diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 9657052fb..a6ebe193e 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -3586,6 +3586,7 @@ public: void addStmtp(AstNode* nodep) { addOp2p(nodep); } // Special accessors bool isJustOneBodyStmt() const { return bodysp() && !bodysp()->nextp(); } + bool isFirstInMyListOfStatements(AstNode* n) const override { return n == bodysp(); } }; class AstAssign final : public AstNodeAssign { @@ -4009,6 +4010,7 @@ public: void condsp(AstNode* nodep) { setOp1p(nodep); } void addBodysp(AstNode* newp) { addOp2p(newp); } bool isDefault() const { return condsp() == nullptr; } + bool isFirstInMyListOfStatements(AstNode* n) const override { return n == bodysp(); } }; class AstSFormatF final : public AstNode { @@ -4694,6 +4696,7 @@ public: virtual bool isGateOptimizable() const override { return false; } virtual int instrCount() const override { return INSTR_COUNT_BRANCH; } virtual bool same(const AstNode* /*samep*/) const override { return true; } + bool isFirstInMyListOfStatements(AstNode* n) const override { return n == bodysp(); } }; class AstRepeat final : public AstNodeStmt { @@ -4711,6 +4714,7 @@ public: } // Not relevant - converted to FOR virtual int instrCount() const override { return INSTR_COUNT_BRANCH; } virtual bool same(const AstNode* /*samep*/) const override { return true; } + bool isFirstInMyListOfStatements(AstNode* n) const override { return n == bodysp(); } }; class AstWait final : public AstNodeStmt { @@ -4722,6 +4726,7 @@ public: } ASTNODE_NODE_FUNCS(Wait) AstNode* bodysp() const { return op3p(); } // op3 = body of loop + bool isFirstInMyListOfStatements(AstNode* n) const override { return n == bodysp(); } }; class AstWhile final : public AstNodeStmt { @@ -4748,6 +4753,7 @@ public: virtual void addBeforeStmt(AstNode* newp, AstNode* belowp) override; // Stop statement searchback here virtual void addNextStmt(AstNode* newp, AstNode* belowp) override; + bool isFirstInMyListOfStatements(AstNode* n) const override { return n == bodysp(); } }; class AstBreak final : public AstNodeStmt { diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 215e282b6..523d208da 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -3090,7 +3090,9 @@ private: newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), nodep->name()); newp->dtypeFrom(adtypep->subDTypep()); - if (!nodep->firstAbovep()) newp->makeStatement(); + // Because queue methods pop_front() or pop_back() can be void cast, + // they use makeStatement to check if they need the c++ ";" added. + if (nodep->isStandaloneBodyStmt()) newp->makeStatement(); } else if (nodep->name() == "push_back" || nodep->name() == "push_front") { methodOkArguments(nodep, 1, 1); methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); diff --git a/test_regress/t/t_void_queue_ops.pl b/test_regress/t/t_void_queue_ops.pl new file mode 100755 index 000000000..b46d46042 --- /dev/null +++ b/test_regress/t/t_void_queue_ops.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(simulator => 1); + +compile( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_void_queue_ops.v b/test_regress/t/t_void_queue_ops.v new file mode 100644 index 000000000..f618cb876 --- /dev/null +++ b/test_regress/t/t_void_queue_ops.v @@ -0,0 +1,190 @@ +module t + (/*AUTOARG*/ + // Inputs + clk + ); + + input clk; + + int cyc = 0; + + + // Test for https://github.com/verilator/verilator/issues/3364 + // Make sure all SV queue API is supported and verilator can generate + // compile-able C++ models for it. + + // simple queue + logic [31:0] my_int_queue [$]; + + // On the functions and tasks, the my_int_queue.pop_[front|back]() call will + // have nodep->firstAbovep() != nullptr. Because the pop_front or pop_back is + // the first node on the "list". + // To fix this, V3Width.cpp will not use firstAbovep(), and instead us + // isStandalongStmt() -- which checks if the pop_front or pop_back is + // 2nd or later, or if it's first in the list that it's in a "block" of code. + // For functions/tasks, that is checked with: + // VN_IS(backp(), NodeFTask)=True, so even though + function automatic void f_pop_back__my_int_queue(); + void'(my_int_queue.pop_back()); + endfunction : f_pop_back__my_int_queue + + function automatic void f_pop_front__my_int_queue(); + void'(my_int_queue.pop_front()); + endfunction : f_pop_front__my_int_queue + + task automatic t_pop_back__my_int_queue(); + void'(my_int_queue.pop_back()); + endtask : t_pop_back__my_int_queue + + task automatic t_pop_front__my_int_queue(); + void'(my_int_queue.pop_front()); + endtask : t_pop_front__my_int_queue + + + task automatic do_random_queue_operation(); + bit [7:0] rand_op; + int rand_index; + logic [31:0] item; + + + rand_op = 8'($urandom_range(32, 0)); + case(rand_op) + 8'd0: ; // nop + + // pushes (2x of these) + 8'd1, 8'd2: my_int_queue.push_back($urandom); + 8'd3, 8'd4: my_int_queue.push_front($urandom); + + // delete: + 8'd5: my_int_queue.delete(); + + // insert(index, item): + 8'd6: begin + rand_index = $urandom_range(my_int_queue.size()); + my_int_queue.insert(rand_index, item); + end + + // shuffle + 8'd7: my_int_queue.shuffle(); + + // Various pops for rand_op >= 8: + // pops to var + // V3Width debug -- firstAbovep()=ASSIGN (which I guess does the ; for us + // so we don't need the queue op to + // do it.) + // isStandalongStmt() will ignore ASSIGN, return false (NodeAssign is + // child of AstNodeStmt) + 8'd8: if (my_int_queue.size() > 0) item = my_int_queue.pop_front(); + 8'd9: if (my_int_queue.size() > 0) item = my_int_queue.pop_back(); + + // pops to the void + // V3Width debug -- firstAbovep()=IF + // This is fixed with isStandalongStmt() -- VN_IS(backp(), NodeIf)=True + 8'd10: if (my_int_queue.size() > 0) void'(my_int_queue.pop_front()); + 8'd11: if (my_int_queue.size() > 0) void'(my_int_queue.pop_back()); + + // pop result to the lhs of a condition, and do something with it. + 8'd12: + if (my_int_queue.size() > 0) + // V3Width debug -- firstAbovep()=LTE (good we don't want a ; here) + if (my_int_queue.pop_front() <= 2022) + my_int_queue.push_front(3022); // living in the year 3022. + + // pop result to the rhs of a condition, and do something with it. + 8'd13: + if (my_int_queue.size() > 0) + // V3Width debug -- firstAbovep()=GT (good we don't want a ; here) + if (4022 > my_int_queue.pop_front()) + my_int_queue.push_front(3023); // living in the year 3023. + + // pops to the void after yet another case: + // V3Width debug -- firstAbovep()=CASEITEM (not a nullptr) + // This is fixed with isStandalongStmt() -- VN_IS(backp(), CaseItem)=True + 8'd14: + case (my_int_queue.size() > 0) + 0: ; + 1: void'(my_int_queue.pop_front()); + default: ; + endcase // case (my_int_queue.size() > 0) + + // V3Width debug -- firstAbovep()=CASEITEM (not a nullptr) + // backp()->nextp()=CASEITEM (different one) + // This is fixed with isStandalongStmt() -- VN_IS(backp(), CaseItem)=True + 8'd15: + case (my_int_queue.size() > 0) + 0: ; + 1: void'(my_int_queue.pop_back()); + default; + endcase // case (my_int_queue.size() > 0) + + // pops in a function or task + 8'd16: if (my_int_queue.size() > 0) f_pop_back__my_int_queue(); + 8'd17: if (my_int_queue.size() > 0) f_pop_front__my_int_queue(); + 8'd18: if (my_int_queue.size() > 0) t_pop_back__my_int_queue(); + 8'd19: if (my_int_queue.size() > 0) t_pop_front__my_int_queue(); + + // But what if we put some dummy code before the pop_back() or pop_front(): + 8'd20: begin + if (my_int_queue.size() > 0) begin + ; // dummy line + // V3Width debug -- firstAbovep()=BEGIN (is not nullptr). + // This is fixed with isStandalongStmt() -- VN_IS(backp(), NodeIf)=True + void'(my_int_queue.pop_back()); + end + end + 8'd21: begin + automatic int temp_int = 0; + if (my_int_queue.size() > 0) begin + temp_int = 5; // dummy line + // V3Width debug -- firstAbovep()=nullptr (good) + void'(my_int_queue.pop_back()); + end + end + 8'd22: begin + if (my_int_queue.size() > 0) begin + automatic int some_temp_dummy_int; + some_temp_dummy_int = 42; + // V3Width debug -- firstAbovep()=nullptr (good) + void'(my_int_queue.pop_back()); + end + end + 8'd23: begin + if (my_int_queue.size() > 0) begin + // no dummy here, just a 'begin' helper before it. + // V3Width debug -- firstAbovep()=BEGIN (is not nullptr). + // This is fixed with isStandalongStmt() -- VN_IS(backp(), NodeIf)=True + void'(my_int_queue.pop_back()); + end + end + + // What about an if of something else, followed by a pop_front? + 8'd24: begin + automatic int temp_int = 0; + if (my_int_queue.size() == 0) begin // dummy + temp_int = 1000; + end + void'(my_int_queue.pop_front()); // firstAbovep() should be nullptr here. + end + + + default: ; // nop + endcase // case (rand_op) + + endtask : do_random_queue_operation + + + + always @ (posedge clk) begin : main + cyc <= cyc + 1; + + do_random_queue_operation(); + + if (cyc > 100) begin + $write("*-* All Finished *-*\n"); + $finish(); + end + end + + + +endmodule : t