From 457ad07ade56c36cb0ed901fac4c6f34e59c57ef Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 12 Jul 2022 17:51:17 +0100 Subject: [PATCH 01/11] Remove unnecessary static state from V3EmitCFunc --- src/V3EmitCFunc.cpp | 64 +++++++++++++++------------------------------ src/V3EmitCFunc.h | 22 ++++++++++++++++ 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/V3EmitCFunc.cpp b/src/V3EmitCFunc.cpp index 2aa4f2342..ae3f8ce03 100644 --- a/src/V3EmitCFunc.cpp +++ b/src/V3EmitCFunc.cpp @@ -169,30 +169,8 @@ void EmitCFunc::emitOpName(AstNode* nodep, const string& format, AstNode* lhsp, } } -// We only do one display at once, so can just use static state -static struct EmitDispState { - string m_format; // "%s" and text from user - std::vector m_argsChar; // Format of each argument to be printed - std::vector m_argsp; // Each argument to be printed - std::vector m_argsFunc; // Function before each argument to be printed - EmitDispState() { clear(); } - void clear() { - m_format = ""; - m_argsChar.clear(); - m_argsp.clear(); - m_argsFunc.clear(); - } - void pushFormat(const string& fmt) { m_format += fmt; } - void pushFormat(char fmt) { m_format += fmt; } - void pushArg(char fmtChar, AstNode* nodep, const string& func) { - m_argsChar.push_back(fmtChar); - m_argsp.push_back(nodep); - m_argsFunc.push_back(func); - } -} emitDispState; - void EmitCFunc::displayEmit(AstNode* nodep, bool isScan) { - if (emitDispState.m_format == "" + if (m_emitDispState.m_format == "" && VN_IS(nodep, Display)) { // not fscanf etc, as they need to return value // NOP } else { @@ -235,12 +213,12 @@ void EmitCFunc::displayEmit(AstNode* nodep, bool isScan) { } else { nodep->v3fatalSrc("Unknown displayEmit node type"); } - ofp()->putsQuoted(emitDispState.m_format); + ofp()->putsQuoted(m_emitDispState.m_format); // Arguments - for (unsigned i = 0; i < emitDispState.m_argsp.size(); i++) { - const char fmt = emitDispState.m_argsChar[i]; - AstNode* const argp = emitDispState.m_argsp[i]; - const string func = emitDispState.m_argsFunc[i]; + for (unsigned i = 0; i < m_emitDispState.m_argsp.size(); i++) { + const char fmt = m_emitDispState.m_argsChar[i]; + AstNode* const argp = m_emitDispState.m_argsp[i]; + const string func = m_emitDispState.m_argsFunc[i]; if (func != "" || argp) { puts(","); ofp()->indentInc(); @@ -265,7 +243,7 @@ void EmitCFunc::displayEmit(AstNode* nodep, bool isScan) { puts(" "); } // Prep for next - emitDispState.clear(); + m_emitDispState.clear(); } } @@ -306,16 +284,16 @@ void EmitCFunc::displayArg(AstNode* dispp, AstNode** elistp, bool isScan, const } else { pfmt = string("%") + vfmt + fmtLetter; } - emitDispState.pushFormat(pfmt); + m_emitDispState.pushFormat(pfmt); if (!ignore) { if (argp->dtypep()->basicp() && argp->dtypep()->basicp()->keyword() == VBasicDTypeKwd::STRING) { // string in SystemVerilog is std::string in C++ which is not POD - emitDispState.pushArg(' ', nullptr, "-1"); + m_emitDispState.pushArg(' ', nullptr, "-1"); } else { - emitDispState.pushArg(' ', nullptr, cvtToStr(argp->widthMin())); + m_emitDispState.pushArg(' ', nullptr, cvtToStr(argp->widthMin())); } - emitDispState.pushArg(fmtLetter, argp, ""); + m_emitDispState.pushArg(fmtLetter, argp, ""); if (fmtLetter == 't' || fmtLetter == '^') { const AstSFormatF* fmtp = nullptr; if (const AstDisplay* const nodep = VN_CAST(dispp, Display)) { @@ -328,10 +306,10 @@ void EmitCFunc::displayArg(AstNode* dispp, AstNode** elistp, bool isScan, const UASSERT_OBJ(fmtp, dispp, "Use of %t must be under AstDisplay, AstSFormat, or AstSFormatF"); UASSERT_OBJ(!fmtp->timeunit().isNone(), fmtp, "timenunit must be set"); - emitDispState.pushArg(' ', nullptr, cvtToStr((int)fmtp->timeunit().powerOfTen())); + m_emitDispState.pushArg(' ', nullptr, cvtToStr((int)fmtp->timeunit().powerOfTen())); } } else { - emitDispState.pushArg(fmtLetter, nullptr, ""); + m_emitDispState.pushArg(fmtLetter, nullptr, ""); } } @@ -341,7 +319,7 @@ void EmitCFunc::displayNode(AstNode* nodep, AstScopeName* scopenamep, const stri // Convert Verilog display to C printf formats // "%0t" becomes "%d" - emitDispState.clear(); + m_emitDispState.clear(); string vfmt; string::const_iterator pos = vformat.begin(); bool inPct = false; @@ -353,7 +331,7 @@ void EmitCFunc::displayNode(AstNode* nodep, AstScopeName* scopenamep, const stri ignore = false; vfmt = ""; } else if (!inPct) { // Normal text - emitDispState.pushFormat(*pos); + m_emitDispState.pushFormat(*pos); } else { // Format character inPct = false; switch (tolower(pos[0])) { @@ -374,7 +352,7 @@ void EmitCFunc::displayNode(AstNode* nodep, AstScopeName* scopenamep, const stri inPct = true; // Get more digits break; case '%': - emitDispState.pushFormat("%%"); // We're printf'ing it, so need to quote the % + m_emitDispState.pushFormat("%%"); // We're printf'ing it, so need to quote the % break; case '*': vfmt += pos[0]; @@ -410,17 +388,17 @@ void EmitCFunc::displayNode(AstNode* nodep, AstScopeName* scopenamep, const stri UASSERT_OBJ(scopenamep, nodep, "Display with %m but no AstScopeName"); const string suffix = scopenamep->scopePrettySymName(); if (suffix == "") { - emitDispState.pushFormat("%S"); + m_emitDispState.pushFormat("%S"); } else { - emitDispState.pushFormat("%N"); // Add a . when needed + m_emitDispState.pushFormat("%N"); // Add a . when needed } - emitDispState.pushArg(' ', nullptr, "vlSymsp->name()"); - emitDispState.pushFormat(suffix); + m_emitDispState.pushArg(' ', nullptr, "vlSymsp->name()"); + m_emitDispState.pushFormat(suffix); break; } case 'l': { // Better than not compiling - emitDispState.pushFormat("----"); + m_emitDispState.pushFormat("----"); break; } default: diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index 305fa47a7..629b90397 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -122,6 +122,28 @@ private: std::vector m_blkChangeDetVec; // All encountered changes in block bool m_emitConstInit = false; // Emitting constant initializer + // State associated with processing $display style string formatting + struct EmitDispState { + string m_format; // "%s" and text from user + std::vector m_argsChar; // Format of each argument to be printed + std::vector m_argsp; // Each argument to be printed + std::vector m_argsFunc; // Function before each argument to be printed + EmitDispState() { clear(); } + void clear() { + m_format = ""; + m_argsChar.clear(); + m_argsp.clear(); + m_argsFunc.clear(); + } + void pushFormat(const string& fmt) { m_format += fmt; } + void pushFormat(char fmt) { m_format += fmt; } + void pushArg(char fmtChar, AstNode* nodep, const string& func) { + m_argsChar.push_back(fmtChar); + m_argsp.push_back(nodep); + m_argsFunc.push_back(func); + } + } m_emitDispState; + protected: EmitCLazyDecls m_lazyDecls; // Visitor for emitting lazy declarations bool m_useSelfForThis = false; // Replace "this" with "vlSelf" From 7e8bafd21726eaf84802863d3d9300236b8a37b5 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 12 Jul 2022 18:37:26 +0100 Subject: [PATCH 02/11] Remove static data use from PartContraction::siblingPairFromRelatives Use std::sort with lambda rather than qsort with static function and static data. Verilation performance neutral. --- src/V3Partition.cpp | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/V3Partition.cpp b/src/V3Partition.cpp index a4177c214..53f101ae3 100644 --- a/src/V3Partition.cpp +++ b/src/V3Partition.cpp @@ -30,6 +30,7 @@ #include "V3Stats.h" #include "V3UniqueNames.h" +#include #include #include #include @@ -1529,20 +1530,6 @@ private: } } - static const GraphWay* s_shortestWaywardCpInclusiveWay; - static int shortestWaywardCpInclusive(const void* vap, const void* vbp) { - const GraphWay* const wp = s_shortestWaywardCpInclusiveWay; - const LogicMTask* const ap = *reinterpret_cast(vap); - const LogicMTask* const bp = *reinterpret_cast(vbp); - const uint32_t aCp = ap->critPathCost(*wp) + ap->stepCost(); - const uint32_t bCp = bp->critPathCost(*wp) + bp->stepCost(); - if (aCp < bCp) return -1; - if (aCp > bCp) return 1; - if (ap->id() < bp->id()) return -1; - if (ap->id() > bp->id()) return 1; - return 0; - } - void siblingPairFromRelatives(GraphWay way, V3GraphVertex* mtaskp, bool exhaustive) { std::vector shortestPrereqs; @@ -1556,10 +1543,13 @@ private: if (shortestPrereqs.empty()) return; - // qsort_r would be nice here, but it isn't portable - s_shortestWaywardCpInclusiveWay = &way; - qsort(&shortestPrereqs[0], shortestPrereqs.size(), sizeof(LogicMTask*), - &shortestWaywardCpInclusive); + std::sort(shortestPrereqs.begin(), shortestPrereqs.end(), + [way](const LogicMTask* ap, const LogicMTask* bp) { + const uint32_t aCp = ap->critPathCost(way) + ap->stepCost(); + const uint32_t bCp = bp->critPathCost(way) + bp->stepCost(); + if (aCp != bCp) return aCp < bCp; + return ap->id() < bp->id(); + }); // Don't make all NxN/2 possible pairs of prereqs, that's a lot // to cart around. Just make a few pairs. @@ -1691,8 +1681,6 @@ private: VL_UNCOPYABLE(PartContraction); }; -const GraphWay* PartContraction::s_shortestWaywardCpInclusiveWay = nullptr; - //###################################################################### // DpiImportCallVisitor From 87f1e06c4187cce7afbbee2e2db1fe12389e5076 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 12 Jul 2022 18:54:17 +0100 Subject: [PATCH 03/11] Small algorithmic improvement of PartContraction::siblingPairFromRelatives Use std::partial_sort for the non-exhaustive case. This is O(n) instead of O(n*log(n)) in the size of the candidate list being sorted. (It actually is O(n*log(k)), but k is constant 6 in the non-exhaustive case). --- src/V3Partition.cpp | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/V3Partition.cpp b/src/V3Partition.cpp index 53f101ae3..6b6aea6d7 100644 --- a/src/V3Partition.cpp +++ b/src/V3Partition.cpp @@ -1541,25 +1541,32 @@ private: if (shortestPrereqs.size() > PART_SIBLING_EDGE_LIMIT) break; } - if (shortestPrereqs.empty()) return; + if (shortestPrereqs.size() <= 1) return; - std::sort(shortestPrereqs.begin(), shortestPrereqs.end(), - [way](const LogicMTask* ap, const LogicMTask* bp) { - const uint32_t aCp = ap->critPathCost(way) + ap->stepCost(); - const uint32_t bCp = bp->critPathCost(way) + bp->stepCost(); - if (aCp != bCp) return aCp < bCp; - return ap->id() < bp->id(); - }); + const auto cmp = [way](const LogicMTask* ap, const LogicMTask* bp) { + const uint32_t aCp = ap->critPathCost(way) + ap->stepCost(); + const uint32_t bCp = bp->critPathCost(way) + bp->stepCost(); + if (aCp != bCp) return aCp < bCp; + return ap->id() < bp->id(); + }; - // Don't make all NxN/2 possible pairs of prereqs, that's a lot - // to cart around. Just make a few pairs. - auto it = shortestPrereqs.cbegin(); - for (unsigned i = 0; exhaustive || (i < 3); ++i) { - if (it == shortestPrereqs.cend()) break; - LogicMTask* const ap = *(it++); - if (it == shortestPrereqs.cend()) break; - LogicMTask* const bp = *(it++); - makeSiblingMC(ap, bp); + // Don't make all possible pairs of prereqs when not requested (non-exhaustive). + // Just make a few pairs. + constexpr size_t MAX_NONEXHAUSTIVE_PAIRS = 3; + + size_t end; // End index of pairs to add to candidates (exclusive) + + if (exhaustive || (shortestPrereqs.size() <= 2 * MAX_NONEXHAUSTIVE_PAIRS)) { + end = shortestPrereqs.size() & ~static_cast(1); // Round down to even + std::sort(shortestPrereqs.begin(), shortestPrereqs.end(), cmp); + } else { + end = 2 * MAX_NONEXHAUSTIVE_PAIRS; + std::partial_sort(shortestPrereqs.begin(), shortestPrereqs.begin() + end, + shortestPrereqs.end(), cmp); + } + + for (size_t i = 0; i < end; i += 2) { + makeSiblingMC(shortestPrereqs[i], shortestPrereqs[i + 1]); } } From 63507e8e29c0c0e1f659d7f7d0cb12205feb8158 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 12 Jul 2022 18:02:45 -0400 Subject: [PATCH 04/11] Internals: Favor UASSERT_OBJ when have object. --- src/V3Ast.cpp | 10 +++++----- src/V3Descope.cpp | 4 ++-- src/V3Inst.cpp | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index a48f701e6..7826214d0 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -281,13 +281,13 @@ void AstNode::addNextHere(AstNode* newp) { // This could be at head, tail, or both (single) // New could be head of single node, or list UASSERT(newp, "Null item passed to addNext"); - UASSERT(!newp->backp(), "New node (back) already assigned?"); + UASSERT_OBJ(!newp->backp(), newp, "New node (back) already assigned?"); debugTreeChange(this, "-addHereThs: ", __LINE__, false); debugTreeChange(newp, "-addHereNew: ", __LINE__, true); newp->editCountInc(); AstNode* const addlastp = newp->m_headtailp; // Last node in list to be added - UASSERT(!addlastp->m_nextp, "Headtailp tail isn't at the tail"); + UASSERT_OBJ(!addlastp->m_nextp, addlastp, "Headtailp tail isn't at the tail"); // Forward links AstNode* const oldnextp = this->m_nextp; @@ -437,7 +437,7 @@ void VNRelinker::dump(std::ostream& str) const { AstNode* AstNode::unlinkFrBackWithNext(VNRelinker* linkerp) { debugTreeChange(this, "-unlinkWNextThs: ", __LINE__, true); AstNode* const oldp = this; - UASSERT(oldp->m_backp, "Node has no back, already unlinked?"); + UASSERT_OBJ(oldp->m_backp, oldp, "Node has no back, already unlinked?"); oldp->editCountInc(); AstNode* const backp = oldp->m_backp; if (linkerp) { @@ -497,7 +497,7 @@ AstNode* AstNode::unlinkFrBackWithNext(VNRelinker* linkerp) { AstNode* AstNode::unlinkFrBack(VNRelinker* linkerp) { debugTreeChange(this, "-unlinkFrBkThs: ", __LINE__, true); AstNode* const oldp = this; - UASSERT(oldp->m_backp, "Node has no back, already unlinked?"); + UASSERT_OBJ(oldp->m_backp, oldp, "Node has no back, already unlinked?"); oldp->editCountInc(); AstNode* const backp = oldp->m_backp; if (linkerp) { @@ -565,7 +565,7 @@ void AstNode::relink(VNRelinker* linkerp) { } AstNode* const newp = this; UASSERT(linkerp && linkerp->m_backp, "Need non-empty linker"); - UASSERT(!newp->backp(), "New node already linked?"); + UASSERT_OBJ(!newp->m_backp, newp, "New node already linked?"); newp->editCountInc(); if (debug() > 8) { diff --git a/src/V3Descope.cpp b/src/V3Descope.cpp index 9e6aea78f..c43c0352f 100644 --- a/src/V3Descope.cpp +++ b/src/V3Descope.cpp @@ -248,8 +248,8 @@ private: VL_RESTORER(m_funcp); if (!nodep->user1()) { // Static functions should have been moved under the corresponding AstClassPackage - UASSERT(!(nodep->isStatic() && VN_IS(m_modp, Class)), - "Static function under AstClass"); + UASSERT_OBJ(!(nodep->isStatic() && VN_IS(m_modp, Class)), nodep, + "Static function under AstClass"); m_funcp = nodep; iterateChildren(nodep); nodep->user1(true); diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index cf3906851..339a5e0a5 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -442,7 +442,7 @@ private: int expr_i = i; if (const AstSliceSel* const slicep = VN_CAST(newp->exprp(), SliceSel)) { varrefp = VN_AS(slicep->fromp(), VarRef); - UASSERT(VN_IS(slicep->rhsp(), Const), "Slices should be constant"); + UASSERT_OBJ(VN_IS(slicep->rhsp(), Const), slicep, "Slices should be constant"); const int slice_index = slicep->declRange().left() + in * slicep->declRange().leftToRightInc(); const auto* const exprArrp = VN_AS(varrefp->dtypep(), UnpackArrayDType); From 108c90038776724e94d8c372554c253b0a4d2341 Mon Sep 17 00:00:00 2001 From: "William D. Jones" Date: Wed, 13 Jul 2022 06:38:03 -0400 Subject: [PATCH 05/11] Fix unique_ptr memory header for MinGW64 (#3493). --- docs/CONTRIBUTORS | 1 + src/V3File.h | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index 6089a5eaa..a80d950b9 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -116,6 +116,7 @@ Unai Martinez-Corral Vassilis Papaefstathiou Veripool API Bot Victor Besyakov +William D. Jones Wilson Snyder Xi Zhang Yoda Lee diff --git a/src/V3File.h b/src/V3File.h index 6c45a0456..6a4ded0de 100644 --- a/src/V3File.h +++ b/src/V3File.h @@ -28,6 +28,7 @@ #include #include #include +#include //============================================================================ // V3File: Create streams, recording dependency information From 178e1789b5169715a4adb9cdd0839cfe6f814859 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 12 Jul 2022 13:34:41 +0100 Subject: [PATCH 06/11] Make AstNode::addHereThisAsNext always O(1) Using unlinkFrBackWithNext is O(n) in the size of the list if unlinking from the middle, so addHereThisAsNext also had this complexity. This patch implements addHereThisAsNext directly, which is always O(1). --- src/V3Ast.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index 7826214d0..5518a98a8 100644 --- a/src/V3Ast.cpp +++ b/src/V3Ast.cpp @@ -631,11 +631,56 @@ void AstNode::relinkOneLink(AstNode*& pointpr, // Ref to pointer that gets set } void AstNode::addHereThisAsNext(AstNode* newp) { - // {old}->this->{next} becomes {old}->new->this->{next} - VNRelinker handle; - this->unlinkFrBackWithNext(&handle); - newp->addNext(this); - handle.relink(newp); + // {back}->this->{next} becomes {back}->new->this->{next} + UASSERT_OBJ(!newp->backp(), newp, "New node already linked?"); + UASSERT_OBJ(this->m_backp, this, "'this' node has no back, already unlinked?"); + UASSERT_OBJ(newp->m_headtailp, newp, "m_headtailp not set on new node"); + // + AstNode* const backp = this->m_backp; + AstNode* const newLastp = newp->m_headtailp; + // + this->editCountInc(); + // Common linkage + newLastp->m_nextp = this; + this->m_backp = newLastp; + newp->m_backp = backp; + // newLastp will not be the last node in the list as 'this' will follow it. + // If newLastp == newp, then below handles newp becoming head + newLastp->m_headtailp = nullptr; + // Linkage dependent on position + if (backp && backp->m_nextp == this) { + // If 'this' is not at the head of a list, then the new node will also not be at the head + // of a list, so we can just link in the new node in the middle. + backp->m_nextp = newp; + newp->m_headtailp = nullptr; + } else { + // If 'this' is at the head of a list, then the new node becomes the head of that list. + if (backp) { + if (backp->m_op1p == this) { + backp->m_op1p = newp; + } else if (backp->m_op2p == this) { + backp->m_op2p = newp; + } else if (backp->m_op3p == this) { + backp->m_op3p = newp; + } else { + UASSERT_OBJ(backp->m_op4p == this, this, "Don't know where newp should go"); + backp->m_op4p = newp; + } + } + // We also need to update m_headtailp. + AstNode* const tailp = this->m_headtailp; + this->m_headtailp = nullptr; + newp->m_headtailp = tailp; + tailp->m_headtailp = newp; + } + // Iterator fixup + if (newLastp->m_iterpp) { + *(newLastp->m_iterpp) = this; + } else if (this->m_iterpp) { + *(this->m_iterpp) = newp; + } + // + debugTreeChange(this, "-addHereThisAsNext: ", __LINE__, true); } void AstNode::swapWith(AstNode* bp) { From e0a38ce2c2b52b1b4ad540d4e535ba7ef36728a4 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Wed, 13 Jul 2022 12:20:30 +0100 Subject: [PATCH 07/11] Remove unnecessary AstNode::clearIter() --- src/V3Ast.h | 1 - src/V3LinkLevel.cpp | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/V3Ast.h b/src/V3Ast.h index 868fc73f8..3b9c88a39 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1787,7 +1787,6 @@ public: void deleteTree(); // Always deletes the next link void checkTree(); // User Interface version void checkIter() const; - void clearIter() { m_iterpp = nullptr; } void dumpPtrs(std::ostream& os = std::cout) const; void dumpTree(std::ostream& os = std::cout, const string& indent = " ", int maxDepth = 0) const; diff --git a/src/V3LinkLevel.cpp b/src/V3LinkLevel.cpp index be8706477..d96892762 100644 --- a/src/V3LinkLevel.cpp +++ b/src/V3LinkLevel.cpp @@ -73,11 +73,7 @@ void V3LinkLevel::modSortByLevel() { // Reorder the netlist's modules to have modules in level sorted order stable_sort(mods.begin(), mods.end(), CmpLevel()); // Sort the vector UINFO(9, "modSortByLevel() sorted\n"); // Comment required for gcc4.6.3 / bug666 - for (AstNodeModule* nodep : mods) { - nodep->clearIter(); // Because we didn't iterate to find the node - // pointers, may have a stale m_iterp() needing cleanup - nodep->unlinkFrBack(); - } + for (AstNodeModule* nodep : mods) nodep->unlinkFrBack(); UASSERT_OBJ(!v3Global.rootp()->modulesp(), v3Global.rootp(), "Unlink didn't work"); for (AstNodeModule* nodep : mods) v3Global.rootp()->addModulep(nodep); UINFO(9, "modSortByLevel() done\n"); // Comment required for gcc4.6.3 / bug666 From 3fc8249429a3cde8c925f1ef6ff87bc82865121e Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Wed, 13 Jul 2022 13:55:40 +0100 Subject: [PATCH 08/11] Use AstNode::addHereThisAsNext in a few places --- src/V3Depth.cpp | 5 +---- src/V3Expand.cpp | 5 +---- src/V3Premit.cpp | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/V3Depth.cpp b/src/V3Depth.cpp index d83d69d66..0c3b791e3 100644 --- a/src/V3Depth.cpp +++ b/src/V3Depth.cpp @@ -68,10 +68,7 @@ private: // Put assignment before the referencing statement AstAssign* const assp = new AstAssign{ nodep->fileline(), new AstVarRef{nodep->fileline(), varp, VAccess::WRITE}, nodep}; - VNRelinker linker2; - m_stmtp->unlinkFrBack(&linker2); - assp->addNext(m_stmtp); - linker2.relink(assp); + m_stmtp->addHereThisAsNext(assp); } // VISITORS diff --git a/src/V3Expand.cpp b/src/V3Expand.cpp index a89d893b6..5c341db26 100644 --- a/src/V3Expand.cpp +++ b/src/V3Expand.cpp @@ -82,10 +82,7 @@ private: static void insertBefore(AstNode* placep, AstNode* newp) { newp->user1(1); // Already processed, don't need to re-iterate - VNRelinker linker; - placep->unlinkFrBack(&linker); - newp->addNext(placep); - linker.relink(newp); + placep->addHereThisAsNext(newp); } static void replaceWithDelete(AstNode* nodep, AstNode* newp) { newp->user1(1); // Already processed, don't need to re-iterate diff --git a/src/V3Premit.cpp b/src/V3Premit.cpp index 836b7c814..50254b1ca 100644 --- a/src/V3Premit.cpp +++ b/src/V3Premit.cpp @@ -109,10 +109,7 @@ private: } else if (m_inTracep) { m_inTracep->addPrecondsp(newp); } else if (m_stmtp) { - VNRelinker linker; - m_stmtp->unlinkFrBack(&linker); - newp->addNext(m_stmtp); - linker.relink(newp); + m_stmtp->addHereThisAsNext(newp); } else { newp->v3fatalSrc("No statement insertion point."); } From 658819bb71553ed82425cf75aa72a92355f4518c Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Wed, 13 Jul 2022 16:01:03 +0100 Subject: [PATCH 09/11] Trivial static const -> constexpr --- src/V3File.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/V3File.cpp b/src/V3File.cpp index 305d19c08..75b2cb5d6 100644 --- a/src/V3File.cpp +++ b/src/V3File.cpp @@ -489,8 +489,8 @@ private: #ifdef INFILTER_PIPE int fd_stdin[2]; int fd_stdout[2]; - static const int P_RD = 0; - static const int P_WR = 1; + constexpr int P_RD = 0; + constexpr int P_WR = 1; if (pipe(fd_stdin) != 0 || pipe(fd_stdout) != 0) { v3fatal("--pipe-filter: Can't pipe: " << strerror(errno)); From f4efcbde5c1a5f700a7db4cfb4e8ffe5e19bf192 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Wed, 13 Jul 2022 16:15:21 +0100 Subject: [PATCH 10/11] Remove simple use of static data from V3OutFormatter::indentSpaces --- src/V3File.cpp | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/V3File.cpp b/src/V3File.cpp index 75b2cb5d6..b19d1f868 100644 --- a/src/V3File.cpp +++ b/src/V3File.cpp @@ -623,17 +623,9 @@ V3OutFormatter::V3OutFormatter(const string& filename, V3OutFormatter::Language //---------------------------------------------------------------------- string V3OutFormatter::indentSpaces(int num) { - // Indent the specified number of spaces. Use spaces. - static char str[MAXSPACE + 20]; - char* cp = str; - if (num > MAXSPACE) num = MAXSPACE; - while (num > 0) { - *cp++ = ' '; - --num; - } - *cp++ = '\0'; - string st{str}; // No const, move optimization - return st; + // Indent the specified number of spaces. + if (num <= 0) return std::string{}; + return std::string(std::min(num, MAXSPACE), ' '); } bool V3OutFormatter::tokenMatch(const char* cp, const char* cmp) { From 3bd830eacf72546382ffbc254ff9d6e0fb38e76f Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Wed, 13 Jul 2022 18:24:48 +0100 Subject: [PATCH 11/11] Minor clean up of initialization --- src/V3Global.cpp | 7 +++++-- src/V3Global.h | 6 +----- src/V3PreShell.cpp | 5 ++--- src/V3PreShell.h | 2 +- src/Verilator.cpp | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/V3Global.cpp b/src/V3Global.cpp index 98129ae78..cb42337e4 100644 --- a/src/V3Global.cpp +++ b/src/V3Global.cpp @@ -27,9 +27,12 @@ #include "V3Stats.h" //###################################################################### -// V3 Class -- top level +// V3Global -AstNetlist* V3Global::makeNetlist() { return new AstNetlist(); } +void V3Global::boot() { + UASSERT(!m_rootp, "call once"); + m_rootp = new AstNetlist(); +} void V3Global::clear() { #ifdef VL_LEAK_CHECK diff --git a/src/V3Global.h b/src/V3Global.h index 84a75d7e2..a7b54e866 100644 --- a/src/V3Global.h +++ b/src/V3Global.h @@ -119,11 +119,7 @@ public: // CONSTRUCTORS V3Global() {} - AstNetlist* makeNetlist(); - void boot() { - UASSERT(!m_rootp, "call once"); - m_rootp = makeNetlist(); - } + void boot(); void clear(); void shutdown(); // Release allocated resorces // ACCESSORS (general) diff --git a/src/V3PreShell.cpp b/src/V3PreShell.cpp index 6d64c4b7f..4b817a53b 100644 --- a/src/V3PreShell.cpp +++ b/src/V3PreShell.cpp @@ -49,9 +49,8 @@ protected: return level; } - void boot(char** env) { + void boot() { // Create the implementation pointer - if (env) {} if (!s_preprocp) { FileLine* const cmdfl = new FileLine(FileLine::commandLineFilename()); s_preprocp = V3PreProc::createPreProc(cmdfl); @@ -162,7 +161,7 @@ VInFilter* V3PreShellImp::s_filterp = nullptr; //###################################################################### // V3PreShell -void V3PreShell::boot(char** env) { V3PreShellImp::s_preImp.boot(env); } +void V3PreShell::boot() { V3PreShellImp::s_preImp.boot(); } bool V3PreShell::preproc(FileLine* fl, const string& modname, VInFilter* filterp, V3ParseImp* parsep, const string& errmsg) { return V3PreShellImp::s_preImp.preproc(fl, modname, filterp, parsep, errmsg); diff --git a/src/V3PreShell.h b/src/V3PreShell.h index 7e26f940f..9c7b73499 100644 --- a/src/V3PreShell.h +++ b/src/V3PreShell.h @@ -32,7 +32,7 @@ class VSpellCheck; class V3PreShell final { // Static class for calling preprocessor public: - static void boot(char** env); + static void boot(); static bool preproc(FileLine* fl, const string& modname, VInFilter* filterp, V3ParseImp* parsep, const string& errmsg); static void preprocInclude(FileLine* fl, const string& modname); diff --git a/src/Verilator.cpp b/src/Verilator.cpp index 97e3393c4..f503ee6b9 100644 --- a/src/Verilator.cpp +++ b/src/Verilator.cpp @@ -716,7 +716,7 @@ int main(int argc, char** argv, char** env) { // Preprocessor // Before command parsing so we can handle -Ds on command line. - V3PreShell::boot(env); + V3PreShell::boot(); // Command option parsing v3Global.opt.bin(argv[0]);