From 242f661644c17bcaef6b3c7b7930e43b1a3ddda2 Mon Sep 17 00:00:00 2001 From: Ryszard Rozak Date: Fri, 30 Jun 2023 09:12:22 +0200 Subject: [PATCH] Fix selects of static members (#4326) --- src/V3AstNodeExpr.h | 5 +- src/V3AstNodes.cpp | 6 ++ src/V3LinkLValue.cpp | 10 +++ src/V3Width.cpp | 69 +++++++++++-------- test_regress/t/t_class_static_member_sel.pl | 21 ++++++ test_regress/t/t_class_static_member_sel.v | 74 +++++++++++++++++++++ 6 files changed, 155 insertions(+), 30 deletions(-) create mode 100755 test_regress/t/t_class_static_member_sel.pl create mode 100644 test_regress/t/t_class_static_member_sel.v diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index 607a6b148..4ffcc4bda 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -1428,6 +1428,7 @@ class AstMemberSel final : public AstNodeExpr { // @astgen op1 := fromp : AstNodeExpr // Don't need the class we are extracting from, as the "fromp()"'s datatype can get us to it string m_name; + VAccess m_access; // Read or write, as in AstNodeVarRef AstVar* m_varp = nullptr; // Post link, variable within class that is target of selection public: AstMemberSel(FileLine* fl, AstNodeExpr* fromp, VFlagChildDType, const string& name) @@ -1448,10 +1449,12 @@ public: void dump(std::ostream& str) const override; string name() const override VL_MT_STABLE { return m_name; } void name(const string& name) override { m_name = name; } + VAccess access() const { return m_access; } + void access(const VAccess& flag) { m_access = flag; } string emitVerilog() override { V3ERROR_NA_RETURN(""); } string emitC() override { V3ERROR_NA_RETURN(""); } bool cleanOut() const override { return true; } - bool same(const AstNode* samep) const override { return true; } // dtype comparison does it + bool same(const AstNode* samep) const override; int instrCount() const override { return widthInstrs(); } AstVar* varp() const { return m_varp; } void varp(AstVar* nodep) { m_varp = nodep; } diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index c766e2b02..10297491b 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1676,6 +1676,12 @@ AstNodeUOrStructDType* AstMemberDType::getChildStructp() const { return VN_CAST(subdtp, NodeUOrStructDType); // Maybe nullptr } +bool AstMemberSel::same(const AstNode* samep) const { + const AstMemberSel* const sp = static_cast(samep); + return sp != nullptr && access() == sp->access() && fromp()->same(sp->fromp()) + && name() == sp->name() && varp()->same(sp->varp()); +} + void AstMemberSel::dump(std::ostream& str) const { this->AstNodeExpr::dump(str); str << " -> "; diff --git a/src/V3LinkLValue.cpp b/src/V3LinkLValue.cpp index e8648fd6a..81b9ae5cb 100644 --- a/src/V3LinkLValue.cpp +++ b/src/V3LinkLValue.cpp @@ -275,6 +275,16 @@ private: iterateAndNextNull(nodep->thsp()); } } + void visit(AstMemberSel* nodep) override { + if (m_setRefLvalue != VAccess::NOCHANGE) { + nodep->access(m_setRefLvalue); + } else { + // It is the only place where the access is set to member select nodes. + // If it doesn't have to be set to WRITE, it means that it is READ. + nodep->access(VAccess::READ); + } + iterateChildren(nodep); + } void visit(AstNodeFTask* nodep) override { VL_RESTORER(m_ftaskp); m_ftaskp = nodep; diff --git a/src/V3Width.cpp b/src/V3Width.cpp index feba2f0fb..68b6126d5 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -2710,31 +2710,7 @@ private: if (AstNodeUOrStructDType* const adtypep = VN_CAST(fromDtp, NodeUOrStructDType)) { if (memberSelStruct(nodep, adtypep)) return; } else if (AstClassRefDType* const adtypep = VN_CAST(fromDtp, ClassRefDType)) { - if (AstNode* const foundp = memberSelClass(nodep, adtypep)) { - if (AstVar* const varp = VN_CAST(foundp, Var)) { - if (!varp->didWidth()) userIterate(varp, nullptr); - nodep->dtypep(foundp->dtypep()); - nodep->varp(varp); - return; - } - if (AstEnumItemRef* const adfoundp = VN_CAST(foundp, EnumItemRef)) { - nodep->replaceWith(adfoundp->cloneTree(false)); - VL_DO_DANGLING(pushDeletep(nodep), nodep); - return; - } - if (AstNodeFTask* const methodp = VN_CAST(foundp, NodeFTask)) { - nodep->replaceWith(new AstMethodCall{nodep->fileline(), - nodep->fromp()->unlinkFrBack(), - nodep->name(), nullptr}); - VL_DO_DANGLING(pushDeletep(nodep), nodep); - return; - } - UINFO(1, "found object " << foundp << endl); - nodep->v3fatalSrc("MemberSel of non-variable\n" - << nodep->warnContextPrimary() << '\n' - << foundp->warnOther() << "... Location of found object\n" - << foundp->warnContextSecondary()); - } + if (memberSelClass(nodep, adtypep)) return; } else if (AstIfaceRefDType* const adtypep = VN_CAST(fromDtp, IfaceRefDType)) { if (AstNode* const foundp = memberSelIface(nodep, adtypep)) { if (AstVar* const varp = VN_CAST(foundp, Var)) { @@ -2773,15 +2749,50 @@ private: nodep->replaceWith(new AstConst{nodep->fileline(), AstConst::BitFalse{}}); VL_DO_DANGLING(pushDeletep(nodep), nodep); } - AstNode* memberSelClass(AstMemberSel* nodep, AstClassRefDType* adtypep) { - // Returns node if ok + bool memberSelClass(AstMemberSel* nodep, AstClassRefDType* adtypep) { + // Returns true if ok // No need to width-resolve the class, as it was done when we did the child AstClass* const first_classp = adtypep->classp(); UASSERT_OBJ(first_classp, nodep, "Unlinked"); for (AstClass* classp = first_classp; classp;) { - if (AstNode* const foundp = classp->findMember(nodep->name())) return foundp; + if (AstNode* const foundp = classp->findMember(nodep->name())) { + if (AstVar* const varp = VN_CAST(foundp, Var)) { + if (!varp->didWidth()) userIterate(varp, nullptr); + if (varp->lifetime().isStatic()) { + // Static fiels are moved outside the class, so they shouldn't be accessed + // by member select on a class object + AstVarRef* const varRefp + = new AstVarRef{nodep->fileline(), varp, nodep->access()}; + varRefp->classOrPackagep(classp); + nodep->replaceWith(varRefp); + VL_DO_DANGLING(pushDeletep(nodep), nodep); + return true; + } + nodep->dtypep(foundp->dtypep()); + nodep->varp(varp); + return true; + } + if (AstEnumItemRef* const adfoundp = VN_CAST(foundp, EnumItemRef)) { + nodep->replaceWith(adfoundp->cloneTree(false)); + VL_DO_DANGLING(pushDeletep(nodep), nodep); + return true; + } + if (AstNodeFTask* const methodp = VN_CAST(foundp, NodeFTask)) { + nodep->replaceWith(new AstMethodCall{nodep->fileline(), + nodep->fromp()->unlinkFrBack(), + nodep->name(), nullptr}); + VL_DO_DANGLING(pushDeletep(nodep), nodep); + return true; + } + UINFO(1, "found object " << foundp << endl); + nodep->v3fatalSrc("MemberSel of non-variable\n" + << nodep->warnContextPrimary() << '\n' + << foundp->warnOther() << "... Location of found object\n" + << foundp->warnContextSecondary()); + } classp = classp->extendsp() ? classp->extendsp()->classp() : nullptr; } + VSpellCheck speller; for (AstClass* classp = first_classp; classp;) { for (AstNode* itemp = classp->membersp(); itemp; itemp = itemp->nextp()) { @@ -2796,7 +2807,7 @@ private: "Member " << nodep->prettyNameQ() << " not found in class " << first_classp->prettyNameQ() << "\n" << (suggest.empty() ? "" : nodep->fileline()->warnMore() + suggest)); - return nullptr; // Caller handles error + return false; // Caller handles error } AstNode* memberSelIface(AstMemberSel* nodep, AstIfaceRefDType* adtypep) { // Returns node if ok diff --git a/test_regress/t/t_class_static_member_sel.pl b/test_regress/t/t_class_static_member_sel.pl new file mode 100755 index 000000000..1aa73f80a --- /dev/null +++ b/test_regress/t/t_class_static_member_sel.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 2022 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_class_static_member_sel.v b/test_regress/t/t_class_static_member_sel.v new file mode 100644 index 000000000..5bfdacfed --- /dev/null +++ b/test_regress/t/t_class_static_member_sel.v @@ -0,0 +1,74 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +class Foo; + static int x = 1; +endclass + +class Bar; + Foo f; + function new; + f = new; + endfunction +endclass + +class Baz; + function static Bar get_bar; + Bar b = new; + return b; + endfunction +endclass + +class IntWrapper; + int x; +endclass + +class Cls; + static IntWrapper iw; + function new; + if (iw == null) iw = new; + endfunction +endclass + +class ExtendCls extends Cls; +endclass + +class Getter1; + function static int get_1; + return 1; + endfunction +endclass + +module t (/*AUTOARG*/ + ); + + initial begin + Foo foo = new; + Bar bar = new; + Baz baz = new; + ExtendCls ec = new; + Getter1 getter1 = new; + + if (foo.x != 1) $stop; + + foo.x = 2; + if (foo.x != 2) $stop; + + bar.f.x = 3; + if (bar.f.x != 3) $stop; + + baz.get_bar().f.x = 4; + if (baz.get_bar().f.x != 4) $stop; + + ec.iw.x = 5; + if (ec.iw.x != 5) $stop; + + if (getter1.get_1 != 1) $stop; + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule