github/verilator
1
0
Fork 1
mirror of https://github.com/verilator/verilator.git synced 2025-01-05 22:27:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

Generate AstGen::checkTreeiter to enforce Ast op*p use

Browse Source 
Use astgen to generate a more thorough version of AstNode::checkTree,
which checks that operands are or consistent structure and type, as
described in the @astgen op directives. Also change checkTree to always
run when --debug-check is given.

Fix discovered fallout.
This commit is contained in:
Geza Lore 2022-09-21 15:24:17 +01:00
parent ce03293128
commit 9949a6cd17
7 changed files with 97 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
src/V3Ast.cpp
View File

@ -999,43 +999,12 @@ bool AstNode::sameTreeIter(const AstNode* node1p, const AstNode* node2p, bool ig
//======================================================================
// Debugging
void AstNode::checkTreeIter(AstNode* backp) {
void AstNode::checkTreeIter(const AstNode* backp) const {
// private: Check a tree and children
UASSERT_OBJ(backp == this->backp(), this, "Back node inconsistent");
if (VN_IS(this, NodeTermop) || VN_IS(this, NodeVarRef)) {
// Termops have a short-circuited iterateChildren, so check usage
UASSERT_OBJ(!(op1p() || op2p() || op3p() || op4p()), this,
"Terminal operation with non-terminals");
}
if (m_op1p) m_op1p->checkTreeIterList(this);
if (m_op2p) m_op2p->checkTreeIterList(this);
if (m_op3p) m_op3p->checkTreeIterList(this);
if (m_op4p) m_op4p->checkTreeIterList(this);
}
void AstNode::checkTreeIterList(AstNode* backp) {
// private: Check a (possible) list of nodes, this is always the head of the list
// Audited to make sure this is never nullptr
AstNode* const headp = this;
const AstNode* tailp = this;
for (AstNode* nodep = headp; nodep; nodep = nodep->nextp()) {
nodep->checkTreeIter(backp);
UASSERT_OBJ(headp == this || !nextp(), this,
"Headtailp should be null in middle of lists");
tailp = nodep;
backp = nodep;
}
UASSERT_OBJ(headp->m_headtailp == tailp, headp, "Tail in headtailp is inconsistent");
UASSERT_OBJ(tailp->m_headtailp == headp, tailp, "Head in headtailp is inconsistent");
}
void AstNode::checkTree() {
if (!debug()) return;
if (this->backp()) {
// Linked tree- check only the passed node
this->checkTreeIter(this->backp());
} else {
this->checkTreeIterList(this->backp());
switch (this->type()) {
#include "V3Ast__gen_op_checks.h"
default: VL_UNREACHABLE; // LCOV_EXCL_LINE
}
}

7
src/V3Ast.h
View File

@ -1470,8 +1470,7 @@ class AstNode VL_NOT_FINAL {
private:
AstNode* cloneTreeIter();
AstNode* cloneTreeIterList();
void checkTreeIter(AstNode* backp);
void checkTreeIterList(AstNode* backp);
void checkTreeIter(const AstNode* backp) const;
bool gateTreeIter() const;
static bool sameTreeIter(const AstNode* node1p, const AstNode* node2p, bool ignNext,
bool gateOnly);
@ -1826,7 +1825,9 @@ public:
// Does tree of this == node2p?, not allowing non-isGateOptimizable
inline bool sameGateTree(const AstNode* node2p) const;
void deleteTree(); // Always deletes the next link
void checkTree(); // User Interface version
void checkTree() const {
if (v3Global.opt.debugCheck()) checkTreeIter(backp());
}
void checkIter() const;
void dumpPtrs(std::ostream& os = std::cout) const;
void dumpTree(std::ostream& os = std::cout, const string& indent = " ",

4
src/V3AstInlines.h
View File

@ -101,10 +101,10 @@ AstPin::AstPin(FileLine* fl, int pinNum, AstVarRef* varname, AstNode* exprp)
AstDpiExportUpdated::AstDpiExportUpdated(FileLine* fl, AstVarScope* varScopep)
: ASTGEN_SUPER_DpiExportUpdated(fl) {
addOp1p(new AstVarRef{fl, varScopep, VAccess::WRITE});
this->varRefp(new AstVarRef{fl, varScopep, VAccess::WRITE});
}
AstVarScope* AstDpiExportUpdated::varScopep() const { return VN_AS(op1p(), VarRef)->varScopep(); }
AstVarScope* AstDpiExportUpdated::varScopep() const { return varRefp()->varScopep(); }
AstPackArrayDType::AstPackArrayDType(FileLine* fl, VFlagChildDType, AstNodeDType* dtp,
AstRange* rangep)

4
src/V3AstNodeDType.h
View File

@ -281,7 +281,7 @@ public:
class AstAssocArrayDType final : public AstNodeDType {
// Associative array data type, ie "[some_dtype]"
// @astgen op1 := childDTypep : Optional[AstNodeDType] // moved to refDTypep() in V3Width
// @astgen op2 := keyChildDTypep : AstNodeDType // the key, which remains here as a pointer
// @astgen op2 := keyChildDTypep : Optional[AstNodeDType]
private:
AstNodeDType* m_refDTypep; // Elements of this type (after widthing)
AstNodeDType* m_keyDTypep; // Keys of this type (after widthing)
@ -996,7 +996,7 @@ public:
bool isCompound() const override { return true; }
};
class AstRefDType final : public AstNodeDType {
// @astgen op1 := typeofp : AstNode
// @astgen op1 := typeofp : Optional[AstNode]
// @astgen op2 := classOrPackageOpp : Optional[AstNode]
// @astgen op3 := paramsp : List[AstPin]
private:

4
src/V3AstNodeMath.h
View File

@ -187,7 +187,6 @@ protected:
public:
ASTGEN_MEMBERS_NodeTermop;
// Know no children, and hot function, so skip iterator for speed
// See checkTreeIter also that asserts no children
// cppcheck-suppress functionConst
void iterateChildren(VNVisitor& v) {}
void dump(std::ostream& str) const override;
@ -339,7 +338,6 @@ public:
AstNodeModule* classOrPackagep() const { return m_classOrPackagep; }
void classOrPackagep(AstNodeModule* nodep) { m_classOrPackagep = nodep; }
// Know no children, and hot function, so skip iterator for speed
// See checkTreeIter also that asserts no children
// cppcheck-suppress functionConst
void iterateChildren(VNVisitor& v) {}
};
@ -825,7 +823,7 @@ class AstImplication final : public AstNodeMath {
// Verilog |-> |=>
// @astgen op1 := lhsp : AstNode
// @astgen op2 := rhsp : AstNode
// @astgen op3 := sentreep : AstSenTree
// @astgen op3 := sentreep : Optional[AstSenTree]
public:
AstImplication(FileLine* fl, AstNode* lhsp, AstNode* rhsp)
: ASTGEN_SUPER_Implication(fl) {

8
src/V3AstNodeOther.h
View File

@ -429,7 +429,7 @@ public:
};
class AstNodeFTaskRef VL_NOT_FINAL : public AstNodeStmt {
// A reference to a task (or function)
// @astgen op1 := namep : AstNode
// @astgen op1 := namep : Optional[AstNode]
// op2 used by some sub-types only
// @astgen op3 := pinsp : List[AstNode]
// @astgen op4 := scopeNamep : Optional[AstScopeName]
@ -1728,7 +1728,7 @@ public:
};
class AstSenItem final : public AstNode {
// Parents: SENTREE
// @astgen op1 := sensp : AstNode // Sensitivity expression
// @astgen op1 := sensp : Optional[AstNode] // Sensitivity expression
VEdgeType m_edgeType; // Edge type
public:
class Combo {}; // for creator type-overload selection
@ -3077,6 +3077,7 @@ public:
};
class AstDpiExportUpdated final : public AstNodeStmt {
// Denotes that the referenced variable may have been updated via a DPI Export
// @astgen op1 := varRefp : AstVarRef
public:
inline AstDpiExportUpdated(FileLine* fl, AstVarScope* varScopep);
ASTGEN_MEMBERS_DpiExportUpdated;
@ -3496,7 +3497,8 @@ class AstTraceDecl final : public AstNodeStmt {
// Trace point declaration
// Separate from AstTraceInc; as a declaration can't be deleted
// Parents: {statement list}
// @astgen op1 := valuep : AstNode // Expressio being traced
// Expression being traced - Moved to AstTraceInc by V3Trace
// @astgen op1 := valuep : Optional[AstNode]
private:
uint32_t m_code = 0; // Trace identifier code; converted to ASCII by trace routines
const string m_showname; // Name of variable

79
src/astgen
View File

@ -904,6 +904,84 @@ def write_macros(filename):
fh.write("\n")
def write_op_checks(filename):
with open_file(filename) as fh:
indent = ""
def emitBlock(pattern, **fmt):
fh.write(textwrap.indent(textwrap.dedent(pattern), indent).format(**fmt))
for node in SortedNodes:
if not node.isLeaf:
continue
emitBlock('''\
case VNType::at{nodeName}: {{
const Ast{nodeName}* const currp = static_cast<const Ast{nodeName}*>(this);
''',
nodeName=node.name)
indent = " "
for n in range(1, 5):
op = node.getOp(n)
emitBlock("// Checking op{n}p\n",n=n)
if op:
name, monad, kind = op
if not monad:
emitBlock('''\
UASSERT_OBJ(currp->{opName}(), currp, "Ast{nodeName} must have non nullptr {opName}()");
UASSERT_OBJ(!currp->{opName}()->nextp(), currp, "Ast{nodeName}::{opName}() cannot have a non nullptr nextp()");
currp->{opName}()->checkTreeIter(currp);
''',
n=n,
nodeName=node.name,
opName=name)
elif monad == "Optional":
emitBlock('''\
if (Ast{kind}* const opp = currp->{opName}()) {{
UASSERT_OBJ(!currp->{opName}()->nextp(), currp, "Ast{nodeName}::{opName}() cannot have a non nullptr nextp()");
opp->checkTreeIter(currp);
}}
''',
n=n,
nodeName=node.name,
opName=name,
kind=kind)
elif monad == "List":
emitBlock('''\
if (const Ast{kind}* const headp = currp->{opName}()) {{
const AstNode* backp = currp;
const Ast{kind}* tailp = headp;
const Ast{kind}* opp = headp;
do {{
opp->checkTreeIter(backp);
UASSERT_OBJ(opp == headp || !opp->nextp() || !opp->m_headtailp, opp, "Headtailp should be null in middle of lists");
backp = tailp = opp;
opp = {next};
}} while (opp);
UASSERT_OBJ(headp->m_headtailp == tailp, headp, "Tail in headtailp is inconsistent");
UASSERT_OBJ(tailp->m_headtailp == headp, tailp, "Head in headtailp is inconsistent");
}}
''',
n=n,
nodeName=node.name,
opName=name,
kind=kind,
next = "VN_AS(opp->nextp(), {kind})".format(kind=kind) if kind != "Node" else "opp->nextp()"
)
else:
sys.exit("Unknown operand type")
else:
emitBlock('''\
UASSERT_OBJ(!currp->op{n}p(), currp, "Ast{nodeName} does not use op{n}p()");
''', n=n, nodeName=node.name)
indent = ""
emitBlock('''\
break;
}}
''')
######################################################################
# main
@ -1005,6 +1083,7 @@ if Args.classes:
write_types("V3Ast__gen_types.h")
write_yystype("V3Ast__gen_yystype.h")
write_macros("V3Ast__gen_macros.h")
write_op_checks("V3Ast__gen_op_checks.h")
for cpt in Args.infiles:
if not re.search(r'.cpp$', cpt):