Skip to content

Commit

Permalink
Internals: Remove m_pkgp and m_classp from WidthVisitor (verilator#4402)
Browse files Browse the repository at this point in the history
  • Loading branch information
kiryk authored Aug 5, 2023
1 parent 9caa79a commit 4afa14b
Show file tree
Hide file tree
Showing 11 changed files with 163 additions and 44 deletions.
4 changes: 2 additions & 2 deletions src/V3AstNodeDType.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ class AstNodeUOrStructDType VL_NOT_FINAL : public AstNodeDType {
private:
// MEMBERS
string m_name; // Name from upper typedef, if any
AstNodeModule* m_classOrPackagep = nullptr; // Package hierarchy
AstNodeModule* m_classOrPackagep = nullptr; // Package it will be emitted with
const int m_uniqueNum;
bool m_packed;
bool m_isFourstate = false; // V3Width computes
Expand Down Expand Up @@ -1053,7 +1053,7 @@ class AstRefDType final : public AstNodeDType {
// Post-width typedefs are removed and point to type directly
AstNodeDType* m_refDTypep = nullptr; // data type pointed to, BELOW the AstTypedef
string m_name; // Name of an AstTypedef
AstNodeModule* m_classOrPackagep = nullptr; // Package hierarchy
AstNodeModule* m_classOrPackagep = nullptr; // Class/package in which it was defined
public:
AstRefDType(FileLine* fl, const string& name)
: ASTGEN_SUPER_RefDType(fl)
Expand Down
8 changes: 4 additions & 4 deletions src/V3AstNodeExpr.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class AstNodeFTaskRef VL_NOT_FINAL : public AstNodeExpr {
// @astgen op3 := pinsp : List[AstNodeExpr]
// @astgen op4 := scopeNamep : Optional[AstScopeName]
AstNodeFTask* m_taskp = nullptr; // [AfterLink] Pointer to task referenced
AstNodeModule* m_classOrPackagep = nullptr; // Package hierarchy
AstNodeModule* m_classOrPackagep = nullptr; // Class/package of the task
string m_name; // Name of variable
string m_dotted; // Dotted part of scope the name()ed task/func is under or ""
string m_inlinedDots; // Dotted hierarchy flattened out
Expand Down Expand Up @@ -449,7 +449,7 @@ class AstNodeVarRef VL_NOT_FINAL : public AstNodeExpr {
VAccess m_access; // Left hand side assignment
AstVar* m_varp; // [AfterLink] Pointer to variable itself
AstVarScope* m_varScopep = nullptr; // Varscope for hierarchy
AstNodeModule* m_classOrPackagep = nullptr; // Package hierarchy
AstNodeModule* m_classOrPackagep = nullptr; // Class/package of the variable
string m_selfPointer; // Output code object pointer (e.g.: 'this')

protected:
Expand Down Expand Up @@ -716,7 +716,7 @@ class AstClassOrPackageRef final : public AstNodeExpr {
private:
string m_name;
// Node not NodeModule to appease some early parser usage
AstNode* m_classOrPackageNodep; // Package hierarchy
AstNode* m_classOrPackageNodep; // Pointer to class/package referenced
public:
AstClassOrPackageRef(FileLine* fl, const string& name, AstNode* classOrPackageNodep,
AstPin* paramsp)
Expand Down Expand Up @@ -1057,7 +1057,7 @@ class AstEmptyQueue final : public AstNodeExpr {
};
class AstEnumItemRef final : public AstNodeExpr {
AstEnumItem* m_itemp; // [AfterLink] Pointer to item
AstNodeModule* m_classOrPackagep; // Package hierarchy
AstNodeModule* m_classOrPackagep; // Class/package in which it was defined
public:
AstEnumItemRef(FileLine* fl, AstEnumItem* itemp, AstNodeModule* classOrPackagep)
: ASTGEN_SUPER_EnumItemRef(fl)
Expand Down
10 changes: 5 additions & 5 deletions src/V3AstNodeOther.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class AstNodeFTask VL_NOT_FINAL : public AstNode {
bool m_recursive : 1; // Recursive or part of recursion
bool m_underGenerate : 1; // Under generate (for warning)
bool m_virtual : 1; // Virtual method in class
bool m_fromStd : 1; // Part of std
bool m_needProcess : 1; // Implements part of a process that allocates std::process
VLifetime m_lifetime; // Lifetime
protected:
AstNodeFTask(VNType t, FileLine* fl, const string& name, AstNode* stmtsp)
Expand All @@ -112,7 +112,7 @@ class AstNodeFTask VL_NOT_FINAL : public AstNode {
, m_recursive{false}
, m_underGenerate{false}
, m_virtual{false}
, m_fromStd{false} {
, m_needProcess{false} {
addStmtsp(stmtsp);
cname(name); // Might be overridden by dpi import/export
}
Expand Down Expand Up @@ -172,8 +172,8 @@ class AstNodeFTask VL_NOT_FINAL : public AstNode {
bool underGenerate() const { return m_underGenerate; }
void isVirtual(bool flag) { m_virtual = flag; }
bool isVirtual() const { return m_virtual; }
void isFromStd(bool flag) { m_fromStd = flag; }
bool isFromStd() const { return m_fromStd; }
void setNeedProcess() { m_needProcess = true; }
bool needProcess() const { return m_needProcess; }
void lifetime(const VLifetime& flag) { m_lifetime = flag; }
VLifetime lifetime() const { return m_lifetime; }
bool isFirstInMyListOfStatements(AstNode* n) const override { return n == stmtsp(); }
Expand Down Expand Up @@ -2138,7 +2138,7 @@ class AstVFile final : public AstNodeFile {
class AstClass final : public AstNodeModule {
// @astgen op4 := extendsp : List[AstClassExtends]
// MEMBERS
AstClassPackage* m_classOrPackagep = nullptr; // Class package this is under
AstClassPackage* m_classOrPackagep = nullptr; // Package it will be emitted with
bool m_extended = false; // Is extension or extended by other classes
bool m_interfaceClass = false; // Interface class
bool m_needRNG = false; // Need RNG, uses srandom/randomize
Expand Down
10 changes: 9 additions & 1 deletion src/V3LinkDot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3152,7 +3152,15 @@ class LinkDotResolveVisitor final : public VNVisitor {
} else if (nodep->name() == "randomize" || nodep->name() == "srandom"
|| nodep->name() == "get_randstate"
|| nodep->name() == "set_randstate") {
// Resolved in V3Width
if (AstClass* const classp = VN_CAST(m_modp, Class)) {
nodep->classOrPackagep(classp);
} else {
nodep->v3error("Calling implicit class method "
<< nodep->prettyNameQ() << " without being under class");
nodep->replaceWith(new AstConst{nodep->fileline(), 0});
VL_DO_DANGLING(pushDeletep(nodep), nodep);
return;
}
} else if (nodep->dotted() == "") {
if (nodep->pli()) {
if (v3Global.opt.bboxSys()) {
Expand Down
2 changes: 1 addition & 1 deletion src/V3Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,7 @@ class TaskVisitor final : public VNVisitor {
}

// Mark the fact that this function allocates std::process
if (nodep->isFromStd() && nodep->name() == "self") cfuncp->setNeedProcess();
if (nodep->needProcess()) cfuncp->setNeedProcess();

// Delete rest of cloned task and return new func
VL_DO_DANGLING(pushDeletep(nodep), nodep);
Expand Down
57 changes: 28 additions & 29 deletions src/V3Width.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,13 @@ class WidthVisitor final : public VNVisitor {
// STATE
VMemberMap memberMap; // Member names cached for fast lookup
WidthVP* m_vup = nullptr; // Current node state
AstClass* m_classp = nullptr; // Current class
const AstCell* m_cellp = nullptr; // Current cell for arrayed instantiations
const AstEnumItem* m_enumItemp = nullptr; // Current enum item
const AstNodeFTask* m_ftaskp = nullptr; // Current function/task
const AstNodeProcedure* m_procedurep = nullptr; // Current final/always
const AstWith* m_withp = nullptr; // Current 'with' statement
const AstFunc* m_funcp = nullptr; // Current function
const AstAttrOf* m_attrp = nullptr; // Current attribute
AstPackage* m_pkgp = nullptr; // Current package
const bool m_paramsOnly; // Computing parameter value; limit operation
const bool m_doGenerate; // Do errors later inside generate statement
int m_dtTables = 0; // Number of created data type tables
Expand Down Expand Up @@ -2630,28 +2628,35 @@ class WidthVisitor final : public VNVisitor {
}
void visit(AstClass* nodep) override {
if (nodep->didWidthAndSet()) return;
// If the package is std::process, set m_process type to VlProcessRef
if (m_pkgp && m_pkgp->name() == "std" && nodep->name() == "process") {
if (AstVar* const varp = VN_CAST(memberMap.findMember(nodep, "m_process"), Var)) {
AstBasicDType* const dtypep = new AstBasicDType{
nodep->fileline(), VBasicDTypeKwd::PROCESS_REFERENCE, VSigning::UNSIGNED};
v3Global.rootp()->typeTablep()->addTypesp(dtypep);
varp->getChildDTypep()->unlinkFrBack();
varp->dtypep(dtypep);

// If the class is std::process
if (nodep->name() == "process") {
AstPackage* const packagep = getItemPackage(nodep);
if (packagep && packagep->name() == "std") {
// Change type of m_process to VlProcessRef
if (AstVar* const varp = VN_CAST(memberMap.findMember(nodep, "m_process"), Var)) {
AstNodeDType* const dtypep = varp->getChildDTypep();
if (!varp->dtypep()) {
VL_DO_DANGLING(pushDeletep(dtypep->unlinkFrBack()), dtypep);
}
AstBasicDType* const newdtypep = new AstBasicDType{
nodep->fileline(), VBasicDTypeKwd::PROCESS_REFERENCE, VSigning::UNSIGNED};
v3Global.rootp()->typeTablep()->addTypesp(newdtypep);
varp->dtypep(newdtypep);
}
// Mark that self requires process instance
if (AstNodeFTask* const ftaskp
= VN_CAST(memberMap.findMember(nodep, "self"), NodeFTask)) {
ftaskp->setNeedProcess();
}
}
}

// Must do extends first, as we may in functions under this class
// start following a tree of extends that takes us to other classes
VL_RESTORER(m_classp);
m_classp = nodep;
userIterateAndNext(nodep->extendsp(), nullptr);
userIterateChildren(nodep, nullptr); // First size all members
}
void visit(AstPackage* nodep) override {
VL_RESTORER(m_pkgp);
m_pkgp = nodep;
userIterateChildren(nodep, nullptr);
}
void visit(AstThisRef* nodep) override {
if (nodep->didWidthAndSet()) return;
nodep->dtypep(iterateEditMoveDTypep(nodep, nodep->childDTypep()));
Expand Down Expand Up @@ -5315,7 +5320,6 @@ class WidthVisitor final : public VNVisitor {
nodep->didWidth(true);
return;
}
if (m_pkgp && m_pkgp->name() == "std") nodep->isFromStd(true);
if (nodep->classMethod() && nodep->name() == "rand_mode") {
nodep->v3error("The 'rand_mode' method is built-in and cannot be overridden"
" (IEEE 1800-2017 18.8)");
Expand Down Expand Up @@ -5592,22 +5596,17 @@ class WidthVisitor final : public VNVisitor {
|| (!nodep->taskp()
&& (nodep->name() == "get_randstate" || nodep->name() == "set_randstate"))) {
// TODO perhaps this should move to V3LinkDot
if (!m_classp) {
nodep->v3error("Calling implicit class method " << nodep->prettyNameQ()
<< " without being under class");
nodep->replaceWith(new AstConst{nodep->fileline(), 0});
VL_DO_DANGLING(pushDeletep(nodep), nodep);
return;
}
AstClass* const classp = VN_CAST(nodep->classOrPackagep(), Class);
UASSERT_OBJ(classp, nodep, "Should have failed in V3LinkDot");
if (nodep->name() == "randomize") {
nodep->taskp(V3Randomize::newRandomizeFunc(m_classp));
nodep->taskp(V3Randomize::newRandomizeFunc(classp));
memberMap.clear();
} else if (nodep->name() == "srandom") {
nodep->taskp(V3Randomize::newSRandomFunc(m_classp));
nodep->taskp(V3Randomize::newSRandomFunc(classp));
memberMap.clear();
} else if (nodep->name() == "get_randstate") {
methodOkArguments(nodep, 0, 0);
m_classp->baseMostClassp()->needRNG(true);
classp->baseMostClassp()->needRNG(true);
v3Global.useRandomizeMethods(true);
AstCExpr* const newp
= new AstCExpr{nodep->fileline(), "__Vm_rng.get_randstate()", 1, true};
Expand All @@ -5620,7 +5619,7 @@ class WidthVisitor final : public VNVisitor {
AstNodeExpr* const expr1p = VN_AS(nodep->pinsp(), Arg)->exprp(); // May edit
iterateCheckString(nodep, "LHS", expr1p, BOTH);
AstNodeExpr* const exprp = VN_AS(nodep->pinsp(), Arg)->exprp();
m_classp->baseMostClassp()->needRNG(true);
classp->baseMostClassp()->needRNG(true);
v3Global.useRandomizeMethods(true);
AstCExpr* const newp
= new AstCExpr{nodep->fileline(), "__Vm_rng.set_randstate(", 1, true};
Expand Down
2 changes: 0 additions & 2 deletions test_regress/t/t_randomize_method_nclass_bad.out
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
%Error: t/t_randomize_method_nclass_bad.v:9:7: Calling implicit class method 'randomize' without being under class
: ... In instance t
9 | randomize(1);
| ^~~~~~~~~
%Error: t/t_randomize_method_nclass_bad.v:10:7: Calling implicit class method 'srandom' without being under class
: ... In instance t
10 | srandom(1);
| ^~~~~~~
%Error: Exiting due to
16 changes: 16 additions & 0 deletions test_regress/t/t_srandom_class_dep.pl
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/env perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2023 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();

ok(1);
1;
50 changes: 50 additions & 0 deletions test_regress/t/t_srandom_class_dep.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// 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

typedef class Cls;

class A;
extern function void method();
endclass

class B;
extern function void method();
endclass

class C;
extern function void method();
endclass

class D;
extern function void method();
endclass

function void A::method();
B obj = new;
obj.method();
endfunction

function void B::method();
this.srandom(0);
endfunction

function void C::method();
this.srandom(0);
endfunction

function void D::method();
C obj = new;
obj.method();
endfunction

module t;
A obj1 = new;
D obj2 = new;
initial begin
obj1.method();
obj2.method();
end
endmodule
24 changes: 24 additions & 0 deletions test_regress/t/t_std_process_self.pl
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2023 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);

lint(
verilator_flags2 => ["--exe --main --timing"],
make_main => 0,
);

lint(
verilator_flags2 => ["--exe --main --timing --DUSE_STD_PREFIX"],
make_main => 0,
);

ok(1);
1;
24 changes: 24 additions & 0 deletions test_regress/t/t_std_process_self.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// 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 task do_something();
`ifdef USE_STD_PREFIX
std::process p;
`else
process p;
`endif
p = process::self();
endtask
endclass

module t();
initial begin
Foo::do_something();
$write("*-* All Finished *-*\n");
$finish;
end
endmodule

0 comments on commit 4afa14b

Please sign in to comment.