From 4cd69f9feb600149decea1765ab0b24ca56e46bf Mon Sep 17 00:00:00 2001 From: Ryszard Rozak Date: Thu, 24 Oct 2024 15:40:54 +0200 Subject: [PATCH] Fix queue element access (#5551) --- src/V3AstNodeExpr.h | 4 +++ src/V3LinkLValue.cpp | 1 + src/V3Task.cpp | 7 +++-- src/V3Width.cpp | 16 ++++++++-- src/V3WidthSel.cpp | 22 ++------------ test_regress/t/t_dynarray_cast_write.py | 18 ++++++++++++ test_regress/t/t_dynarray_cast_write.v | 39 +++++++++++++++++++++++++ test_regress/t/t_queue_output_func.py | 18 ++++++++++++ test_regress/t/t_queue_output_func.v | 27 +++++++++++++++++ 9 files changed, 126 insertions(+), 26 deletions(-) create mode 100755 test_regress/t/t_dynarray_cast_write.py create mode 100644 test_regress/t/t_dynarray_cast_write.v create mode 100755 test_regress/t/t_queue_output_func.py create mode 100644 test_regress/t/t_queue_output_func.v diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index 53881bd89..a883caa2f 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -4424,6 +4424,8 @@ class AstSelBit final : public AstNodePreSel { // Single bit range extraction, perhaps with non-constant selection or array selection // Gets replaced during link with AstArraySel or AstSel // @astgen alias op2 := bitp +private: + VAccess m_access; // Left hand side assignment public: AstSelBit(FileLine* fl, AstNodeExpr* fromp, AstNodeExpr* bitp) : ASTGEN_SUPER_SelBit(fl, fromp, bitp, nullptr) { @@ -4431,6 +4433,8 @@ public: "not coded to create after dtypes resolved"); } ASTGEN_MEMBERS_AstSelBit; + VAccess access() const { return m_access; } + void access(const VAccess& flag) { m_access = flag; } }; class AstSelExtract final : public AstNodePreSel { // Range extraction, gets replaced with AstSel diff --git a/src/V3LinkLValue.cpp b/src/V3LinkLValue.cpp index 01d207de7..e0d14cdfb 100644 --- a/src/V3LinkLValue.cpp +++ b/src/V3LinkLValue.cpp @@ -311,6 +311,7 @@ class LinkLValueVisitor final : public VNVisitor { } } void visit(AstNodePreSel* nodep) override { + if (AstSelBit* const selbitp = VN_CAST(nodep, SelBit)) selbitp->access(m_setRefLvalue); VL_RESTORER(m_setRefLvalue); { // Only set lvalues on the from iterateAndNextNull(nodep->fromp()); diff --git a/src/V3Task.cpp b/src/V3Task.cpp index f96ab727f..460647692 100644 --- a/src/V3Task.cpp +++ b/src/V3Task.cpp @@ -493,10 +493,11 @@ class TaskVisitor final : public VNVisitor { || VN_IS(pinp, ArraySel)) { refArgOk = true; } else if (AstCMethodHard* const cMethodp = VN_CAST(pinp, CMethodHard)) { - refArgOk = cMethodp->name() == "at" || cMethodp->name() == "atBack"; if (VN_IS(cMethodp->fromp()->dtypep()->skipRefp(), QueueDType)) { - cMethodp->name(cMethodp->name() == "at" ? "atWriteAppend" - : "atWriteAppendBack"); + refArgOk = cMethodp->name() == "atWriteAppend" + || cMethodp->name() == "atWriteAppendBack"; + } else { + refArgOk = cMethodp->name() == "at" || cMethodp->name() == "atBack"; } } if (refArgOk) { diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 9a062f009..8220b133a 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -3520,9 +3520,19 @@ class WidthVisitor final : public VNVisitor { return VN_AS(nodep->pinsp(), Arg)->exprp(); } void methodCallLValueRecurse(AstMethodCall* nodep, AstNode* childp, const VAccess& access) { - if (const AstCMethodHard* const ichildp = VN_CAST(childp, CMethodHard)) { - if (ichildp->name() == "at" || ichildp->name() == "atWrite" - || ichildp->name() == "atWriteAppend" || ichildp->name() == "atWriteAppendBack") { + if (AstCMethodHard* const ichildp = VN_CAST(childp, CMethodHard)) { + const std::string name = ichildp->name(); + if (name == "at" || name == "atWrite" || name == "atBack" || name == "atWriteAppend" + || name == "atWriteAppendBack") { + const AstNodeDType* const fromDtypep = ichildp->fromp()->dtypep()->skipRefp(); + if (VN_IS(fromDtypep, QueueDType) || VN_IS(fromDtypep, DynArrayDType)) { + // Change access methods to writable ones + if (name == "at") { + ichildp->name("atWrite"); + } else if (name == "atBack") { + ichildp->name("atWriteAppendBack"); + } + } methodCallLValueRecurse(nodep, ichildp->fromp(), access); return; } diff --git a/src/V3WidthSel.cpp b/src/V3WidthSel.cpp index 24d7243c3..1ebfa828a 100644 --- a/src/V3WidthSel.cpp +++ b/src/V3WidthSel.cpp @@ -215,24 +215,6 @@ class WidthSelVisitor final : public VNVisitor { } } - static bool isPossibleWrite(AstNodeExpr* nodep) { - AstNode* abovep = nodep->firstAbovep(); - if (AstNodeAssign* const assignp = VN_CAST(abovep, NodeAssign)) { - // On an assign LHS, assume a write - return assignp->lhsp() == nodep; - } - if (AstMethodCall* const methodCallp = VN_CAST(abovep, MethodCall)) { - // A method call can write - return methodCallp->fromp() == nodep; - } - if (AstNodePreSel* const preSelp = VN_CAST(abovep, NodePreSel)) { - // If we're not selected from, it's not a write (we're the index) - if (preSelp->fromp() != nodep) return false; - } - AstNodeExpr* exprp = VN_CAST(abovep, NodeExpr); - return exprp ? isPossibleWrite(exprp) : false; - } - // VISITORS // If adding new visitors, ensure V3Width's visit(TYPE) calls into here @@ -300,7 +282,7 @@ class WidthSelVisitor final : public VNVisitor { VL_DO_DANGLING(pushDeletep(nodep), nodep); } else if (const AstDynArrayDType* const adtypep = VN_CAST(ddtypep, DynArrayDType)) { // SELBIT(array, index) -> CMETHODCALL(queue, "at", index) - const char* methodName = isPossibleWrite(nodep) ? "atWrite" : "at"; + const char* methodName = nodep->access().isWriteOrRW() ? "atWrite" : "at"; AstCMethodHard* const newp = new AstCMethodHard{nodep->fileline(), fromp, methodName, rhsp}; newp->dtypeFrom(adtypep->subDTypep()); // Need to strip off queue reference @@ -310,7 +292,7 @@ class WidthSelVisitor final : public VNVisitor { } else if (const AstQueueDType* const adtypep = VN_CAST(ddtypep, QueueDType)) { // SELBIT(array, index) -> CMETHODCALL(queue, "at", index) AstCMethodHard* newp; - const char* methodName = isPossibleWrite(nodep) ? "atWriteAppend" : "at"; + const char* methodName = nodep->access().isWriteOrRW() ? "atWriteAppend" : "at"; if (AstNodeExpr* const backnessp = selQueueBackness(rhsp)) { newp = new AstCMethodHard{nodep->fileline(), fromp, std::string(methodName) + "Back", backnessp}; diff --git a/test_regress/t/t_dynarray_cast_write.py b/test_regress/t/t_dynarray_cast_write.py new file mode 100755 index 000000000..d4f986441 --- /dev/null +++ b/test_regress/t/t_dynarray_cast_write.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_dynarray_cast_write.v b/test_regress/t/t_dynarray_cast_write.v new file mode 100644 index 000000000..3cf66f7a8 --- /dev/null +++ b/test_regress/t/t_dynarray_cast_write.v @@ -0,0 +1,39 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2024 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +class Foo; + int x = 1; +endclass + +class Bar extends Foo; + function new; + x = 2; + endfunction +endclass + +module t (/*AUTOARG*/); + initial begin + int sel_bit = 3; + Bar bar = new; + Foo foo = bar; + Bar bars[] = new[4]; + $cast(bars[0], foo); + if (bars[0].x != 2) $stop; + + $cast(bars[sel_bit[0]], foo); + if (bars[1].x != 2) $stop; + + $cast(bars[bars[0].x], foo); + if (bars[2].x != 2) $stop; + + $cast(bars[sel_bit[1:0]], foo); + if (bars[3].x != 2) $stop; + + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule diff --git a/test_regress/t/t_queue_output_func.py b/test_regress/t/t_queue_output_func.py new file mode 100755 index 000000000..d4f986441 --- /dev/null +++ b/test_regress/t/t_queue_output_func.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile() + +test.execute() + +test.passes() diff --git a/test_regress/t/t_queue_output_func.v b/test_regress/t/t_queue_output_func.v new file mode 100644 index 000000000..eb92638ab --- /dev/null +++ b/test_regress/t/t_queue_output_func.v @@ -0,0 +1,27 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2024 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +class Cls; + int x = 1; +endclass + +task init_set_2 (output Cls c); + c = new; + c.x = 2; +endtask + +module t (/*AUTOARG*/); + + initial begin + Cls cls_q[$]; + init_set_2(cls_q[0]); + if (cls_q[0].x != 2) $stop; + + $write("*-* All Finished *-*\n"); + $finish; + end + +endmodule