From 5d98c597e1784bc831d05ad1a2c1df290ec718ee Mon Sep 17 00:00:00 2001 From: Todd Strader Date: Wed, 17 Dec 2025 10:25:22 -0500 Subject: [PATCH] #6707 cleanup --- src/V3AstNodeDType.h | 8 -- src/V3AstNodeOther.h | 6 +- src/V3AstNodeStmt.h | 2 +- src/V3AstNodes.cpp | 77 +++++++------------- src/V3EmitCBase.cpp | 2 +- src/V3Trace.cpp | 14 +--- src/V3TraceDecl.cpp | 22 ++---- test_regress/t/t_cast_signed.v | 1 - test_regress/t/t_trace_type_dupes_structs.py | 1 - 9 files changed, 40 insertions(+), 93 deletions(-) diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index cb4a1594f..571f35286 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -129,7 +129,6 @@ public: // Ideally an IEEE $typename virtual string prettyDTypeName(bool) const { return prettyTypeName(); } string prettyDTypeNameQ() const { return "'" + prettyDTypeName(false) + "'"; } - virtual string cDTypeName() const { return ""; } // // Changing the width may confuse the data type resolution, so must clear // TypeTable cache after use. @@ -272,7 +271,6 @@ public: void dump(std::ostream& str) const override; void dumpJson(std::ostream& str) const override; string prettyDTypeName(bool) const override; - string cDTypeName() const override; bool isCompound() const override { return !packed(); } // For basicp() we reuse the size to indicate a "fake" basic type of same size AstBasicDType* basicp() const override VL_MT_STABLE { @@ -447,7 +445,6 @@ public: bool similarDTypeNode(const AstNodeDType* samep) const override; string name() const override VL_MT_STABLE { return m.m_keyword.ascii(); } string prettyDTypeName(bool full) const override; - string cDTypeName() const override; const char* broken() const override { BROKEN_RTN(dtypep() != this); BROKEN_RTN(v3Global.widthMinUsage() == VWidthMinUsage::VERILOG_WIDTH @@ -1237,9 +1234,6 @@ public: string prettyDTypeName(bool full) const override { return subDTypep() ? prettyName(subDTypep()->prettyDTypeName(full)) : prettyName(); } - string cDTypeName() const override { - return subDTypep() ? subDTypep()->cDTypeName() : cDTypeName(); - } AstBasicDType* basicp() const override VL_MT_STABLE { return subDTypep() ? subDTypep()->basicp() : nullptr; } @@ -1451,7 +1445,6 @@ public: inline AstPackArrayDType(FileLine* fl, AstNodeDType* dtp, AstRange* rangep); ASTGEN_MEMBERS_AstPackArrayDType; string prettyDTypeName(bool full) const override; - string cDTypeName() const override; bool isCompound() const override { return false; } }; class AstUnpackArrayDType final : public AstNodeArrayDType { @@ -1479,7 +1472,6 @@ public: } ASTGEN_MEMBERS_AstUnpackArrayDType; string prettyDTypeName(bool full) const override; - string cDTypeName() const override; bool sameNode(const AstNode* samep) const override { const AstUnpackArrayDType* const sp = VN_DBG_AS(samep, UnpackArrayDType); return m_isCompound == sp->m_isCompound; diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index e39a891bb..d74c188d1 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -2030,6 +2030,10 @@ public: } VDirection direction() const VL_MT_SAFE { return m_direction; } bool isIO() const VL_MT_SAFE { return m_direction != VDirection::NONE; } + bool isVLIO() const { + const AstBasicDType* const bdtypep = basicp(); + return isIO() && bdtypep && !bdtypep->isOpaque(); + } void declDirection(const VDirection& flag) { m_declDirection = flag; } VDirection declDirection() const { return m_declDirection; } void varType(VVarType type) { m_varType = type; } @@ -2262,7 +2266,7 @@ class AstVarScope final : public AstNode { // @astgen ptr := m_varp : Optional[AstVar] // [AfterLink] Pointer to variable itself bool m_trace : 1; // Tracing is turned on for this scope bool m_optimizeLifePost : 1; // One half of an NBA pair using ShadowVariable scheme. Optimize. - // NOCOMMIT -- is this the right way? + // NOCOMMIT -- is this the right way? bool m_tracePreserve : 1; // Preserve for trace logic public: AstVarScope(FileLine* fl, AstScope* scopep, AstVar* varp) diff --git a/src/V3AstNodeStmt.h b/src/V3AstNodeStmt.h index 3bb3cf7d3..d50bd513f 100644 --- a/src/V3AstNodeStmt.h +++ b/src/V3AstNodeStmt.h @@ -1206,7 +1206,7 @@ class AstTraceDecl final : public AstNodeStmt { const VNumRange m_arrayRange; // Property of var the trace details const VVarType m_varType; // Type of variable (for localparam vs. param) const VDirection m_declDirection; // Declared direction input/output etc - // NOCOMMIT -- pretty sure something isn't needed here + // NOCOMMIT -- pretty sure something isn't needed here const bool m_inDtypeFunc; // Trace decl inside type init function string m_dtypeParamName; // Parameter name for type functions int m_codeInc{0}; // Code increment for type diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 35d0ec859..a2d24be29 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -748,33 +748,34 @@ string AstVar::cPubArgType(bool named, bool forReturn) const { return arg; } -class dpiTypesToStringConverter VL_NOT_FINAL { -public: - virtual string openArray(const AstVar*) const { return "const svOpenArrayHandle"; } - virtual string bitLogicVector(const AstVar* /*varp*/, bool isBit) const { - return isBit ? "svBitVecVal" : "svLogicVecVal"; - } - virtual string primitive(const AstVar* varp) const { - string type; - const VBasicDTypeKwd keyword = varp->basicp()->keyword(); - if (keyword.isDpiUnsignable() && !varp->basicp()->isSigned()) type = "unsigned "; - type += keyword.dpiType(); - return type; - } - string convert(const AstVar* varp) const { - if (varp->isDpiOpenArray()) { - return openArray(varp); - } else if (const AstBasicDType* const basicp = varp->basicp()) { - if (basicp->isDpiBitVec() || basicp->isDpiLogicVec()) { - return bitLogicVector(varp, basicp->isDpiBitVec()); - } else { - return primitive(varp); - } +class dpiTypesToStringConverter VL_NOT_FINAL{public : virtual string openArray(const AstVar*) + const {return "const svOpenArrayHandle"; +} +virtual string bitLogicVector(const AstVar* /*varp*/, bool isBit) const { + return isBit ? "svBitVecVal" : "svLogicVecVal"; +} +virtual string primitive(const AstVar* varp) const { + string type; + const VBasicDTypeKwd keyword = varp->basicp()->keyword(); + if (keyword.isDpiUnsignable() && !varp->basicp()->isSigned()) type = "unsigned "; + type += keyword.dpiType(); + return type; +} +string convert(const AstVar* varp) const { + if (varp->isDpiOpenArray()) { + return openArray(varp); + } else if (const AstBasicDType* const basicp = varp->basicp()) { + if (basicp->isDpiBitVec() || basicp->isDpiLogicVec()) { + return bitLogicVector(varp, basicp->isDpiBitVec()); } else { - return "UNKNOWN"; + return primitive(varp); } + } else { + return "UNKNOWN"; } -}; +} +} +; string AstVar::dpiArgType(bool named, bool forReturn) const { if (forReturn) { @@ -1706,14 +1707,6 @@ string AstBasicDType::prettyDTypeName(bool) const { } return os.str(); } -string AstBasicDType::cDTypeName() const { - std::ostringstream os; - os << keyword().ascii(); - if (isRanged() && !rangep() && keyword().width() <= 1) { - os << "__BRA__" << cLeft() << "__" << cRight() << "__KET__"; - } - return os.str(); -} void AstNodeExpr::dump(std::ostream& str) const { this->AstNode::dump(str); } void AstNodeExpr::dumpJson(std::ostream& str) const { dumpJsonGen(str); } @@ -2376,7 +2369,6 @@ string AstNodeUOrStructDType::prettyDTypeName(bool full) const { result += "}" + prettyName(); return result; } -string AstNodeUOrStructDType::cDTypeName() const { return verilogKwd() + "__" + name(); } void AstNodeDType::dump(std::ostream& str) const { this->AstNode::dump(str); if (generic()) str << " [GENERIC]"; @@ -2435,25 +2427,6 @@ string AstUnpackArrayDType::prettyDTypeName(bool full) const { os << subp->prettyDTypeName(full) << "$" << ranges; return os.str(); } -// NOCOMMIT -- copypastaed from prettyDTypeName() -- is there a better way? encodeName()? name()? -string AstPackArrayDType::cDTypeName() const { - std::ostringstream os; - if (const auto subp = subDTypep()) os << subp->cDTypeName(); - os << "__BRA__" + cLeft() + "__" + cRight() + "__KET__"; - return os.str(); -} -string AstUnpackArrayDType::cDTypeName() const { - std::ostringstream os; - string ranges = "__BRA__" + cLeft() + "__" + cRight() + "__KET__"; - // See above re: $ - AstNodeDType* subp = subDTypep()->skipRefp(); - while (AstUnpackArrayDType* adtypep = VN_CAST(subp, UnpackArrayDType)) { - ranges += "__BRA__" + adtypep->cLeft() + "__" + adtypep->cRight() + "__KET__"; - subp = adtypep->subDTypep()->skipRefp(); - } - os << subp->cDTypeName() << "__024__" << ranges; - return os.str(); -} std::vector AstUnpackArrayDType::unpackDimensions() { std::vector dims; for (AstUnpackArrayDType* unpackp = this; unpackp;) { diff --git a/src/V3EmitCBase.cpp b/src/V3EmitCBase.cpp index 50a2ec195..017ad9806 100644 --- a/src/V3EmitCBase.cpp +++ b/src/V3EmitCBase.cpp @@ -197,7 +197,7 @@ void EmitCBaseVisitorConst::emitVarDecl(const AstVar* nodep, bool asRef) { if (asRef && refNeedParens) puts(")"); emitDeclArrayBrackets(nodep); puts(";\n"); - } else if (nodep->isIO() && basicp && !basicp->isOpaque()) { + } else if (nodep->isVLIO()) { if (nodep->isInout()) { putns(nodep, "VL_INOUT"); } else if (nodep->isWritable()) { diff --git a/src/V3Trace.cpp b/src/V3Trace.cpp index ca7cbfae0..5f3a3345a 100644 --- a/src/V3Trace.cpp +++ b/src/V3Trace.cpp @@ -52,7 +52,6 @@ VL_DEFINE_DEBUG_FUNCTIONS; -// NOCOMMIT -- do runtime bake off //###################################################################### // Graph vertexes @@ -514,8 +513,7 @@ class TraceVisitor final : public VNVisitor { funcName += "_sub"; } if (declp) { - funcName += "_dtype_"; - funcName += declp->valuep()->dtypep()->cDTypeName(); + funcName += "_dtype__"; funcName = m_dtypeNames.get(funcName); } else { funcName += "_"; @@ -680,7 +678,6 @@ class TraceVisitor final : public VNVisitor { declp->dtypeVscp(nullptr); - // NOCOMMIT -- ???? subStmts += 1; } else { AstTraceInc* const incp = new AstTraceInc{flp, declp, VTraceType::CONSTANT}; @@ -771,7 +768,7 @@ class TraceVisitor final : public VNVisitor { ifp = nullptr; } - // NOCOMMIT -- is it OK to do this only on the aggregate signal? + // NOCOMMIT -- is it OK to do this only on the aggregate signal? // If required, create the conditional node checking the activity flags if (!prevActSet || actSet != *prevActSet) { FileLine* const flp = m_topScopep->fileline(); @@ -809,17 +806,12 @@ class TraceVisitor final : public VNVisitor { argsp, new AstVarRef{flp, declp->dtypeVscp(), VAccess::READ}); AstCCall* const callChgp = new AstCCall{flp, funcs.chgFuncp, argsp}; callChgp->dtypeSetVoid(); - callChgp->argTypes(callChgp->argTypes() - + "bufp, " - // NOCOMMIT -- some kind of two-wrongs off-by-one error - // somewhere: really seems like it should be code() - 1 for - // the chg func (see old chg func) + callChgp->argTypes(callChgp->argTypes() + "bufp, " + std::to_string(declp->code())); ifp->addThensp(callChgp->makeStmt()); declp->dtypeVscp(nullptr); - // NOCOMMIT -- ???? subStmts += 2; } else { AstTraceInc* const incFulp = new AstTraceInc{flp, declp, VTraceType::FULL}; diff --git a/src/V3TraceDecl.cpp b/src/V3TraceDecl.cpp index 3e862fedb..6a3ff1f7d 100644 --- a/src/V3TraceDecl.cpp +++ b/src/V3TraceDecl.cpp @@ -271,7 +271,7 @@ class TraceDeclVisitor final : public VNVisitor { flp, m_traName, m_traVscp->varp(), valuep, bitRange, arrayRange, dtypeCallp, dtypeCallp ? m_traVscp : nullptr, m_offset != 0}; - // NOCOMMIT -- m_offset and may be redundant with something else here ^ + // NOCOMMIT -- m_offset and may be redundant with something else here ^ if (m_offset) { newp->code(m_offset); if (!dtypeCallp) { m_offset += newp->codeInc(); } @@ -284,12 +284,10 @@ class TraceDeclVisitor final : public VNVisitor { }); } if (dtypeCallp) { - // NOCOMMIT -- are both necessary? + // NOCOMMIT -- are both necessary? m_traVscp->tracePreserve(true); m_traVscp->varp()->trace(true); - m_traVscp->varp()->sigPublic(true); // NOCOMMIT -- this is a lie -- FIX - // NOCOMMIT -- adding this because const and non-const func param names conflict -- - // probably a better way + m_traVscp->varp()->sigPublic(true); // NOCOMMIT -- this is a lie -- FIX newp->dtypeParamName(VN_AS(dtypeCallp->funcp()->user2p(), VarScope) ->varp() ->vlArgType(true, false, true, "", true, true)); @@ -581,10 +579,7 @@ class TraceDeclVisitor final : public VNVisitor { auto pair = m_dtypeFuncs.emplace(skipTypep, nullptr); AstCFunc** funcpp = &pair.first->second; if (pair.second) { - string dtypeName = skipTypep->cDTypeName(); - const string name{"trace_init_dtype__" + dtypeName}; - // NOCOMMIT -- should we only use V3UniqueNames instead of worrying about cDTypeName()? - *funcpp = newCFunc(flp, m_dtypeNames.get(name)); + *funcpp = newCFunc(flp, m_dtypeNames.get("trace_init_dtype__")); (*funcpp)->user2p(m_traVscp); } @@ -613,7 +608,6 @@ class TraceDeclVisitor final : public VNVisitor { UASSERT_OBJ(false, skipTypep, "Creating a trace function for an unexpected type"); } // Code 0 is a sentinel value - // NOCOMMIT -- handle that ^ so we don't need the -1's?' m_dtypeFunc->user1(m_offset - 1); } @@ -655,13 +649,7 @@ class TraceDeclVisitor final : public VNVisitor { addToSubFunc(new AstTracePopPrefix{flp}); } - // NOCOMMIT -- how to handle VL_* macro'ed types? - bool isBasicIO() { - const AstVar* varp = m_traVscp->varp(); - const AstBasicDType* basicp = varp->basicp(); - // NOCOMMIT -- lifted from V3EmitCBase -- AstVar method? - return varp->isIO() && basicp && !basicp->isOpaque(); - } + bool isBasicIO() { return m_traVscp->varp()->isVLIO(); } void visit(AstUnpackArrayDType* nodep) override { // Note more specific dtypes above if (!m_traVscp) return; diff --git a/test_regress/t/t_cast_signed.v b/test_regress/t/t_cast_signed.v index d9cc53979..70e0259b1 100644 --- a/test_regress/t/t_cast_signed.v +++ b/test_regress/t/t_cast_signed.v @@ -13,7 +13,6 @@ module t; initial begin smaller = 8'hfa; bigger = bigger_t'(signed'(smaller)); - $display("%x", bigger); // NOCOMMIT if (bigger != 16'hfffa) $stop; $write("*-* All Finished *-*\n"); diff --git a/test_regress/t/t_trace_type_dupes_structs.py b/test_regress/t/t_trace_type_dupes_structs.py index bb8b460f3..3ed763275 100755 --- a/test_regress/t/t_trace_type_dupes_structs.py +++ b/test_regress/t/t_trace_type_dupes_structs.py @@ -11,7 +11,6 @@ import vltest_bootstrap test.scenarios("simulator_st") test.top_filename = "t/t_trace_type_dupes.v" -test.sim_time = 2000000 # NOCOMMIT -- for benchmarking, leave in? test.compile( # artificially low trace splitting for force cross-split type function usage