diff --git a/src/V3SplitVar.cpp b/src/V3SplitVar.cpp index b06f0ec76..2f96ecbdd 100644 --- a/src/V3SplitVar.cpp +++ b/src/V3SplitVar.cpp @@ -127,14 +127,12 @@ struct SplitVarImpl { static AstNodeAssign* newAssign(FileLine* fileline, AstNode* lhsp, AstNode* rhsp, const AstVar* varp) { if (varp->isFuncLocal() || varp->isFuncReturn()) { - return new AstAssign(fileline, lhsp, rhsp); + return new AstAssign{fileline, lhsp, rhsp}; } else { - return new AstAssignW(fileline, lhsp, rhsp); + return new AstAssignW{fileline, lhsp, rhsp}; } } - static const char* const notSplitMsg; - // These check functions return valid pointer to the reason text if a variable cannot be split. // Check if a var type can be split @@ -152,10 +150,12 @@ struct SplitVarImpl { return nullptr; } - static const char* cannotSplitConnectedPortReason(AstPin* pinp) { - AstVar* varp = pinp->modVarp(); + static const char* cannotSplitConnectedPortReason(const AstPin* pinp) { + const AstVar* const varp = pinp->modVarp(); if (!varp) return "it is not connected"; - if (const char* reason = cannotSplitVarDirectionReason(varp->direction())) return reason; + if (const char* const reason = cannotSplitVarDirectionReason(varp->direction())) { + return reason; + } return nullptr; } @@ -167,11 +167,15 @@ struct SplitVarImpl { } static const char* cannotSplitVarCommonReason(const AstVar* varp) { - if (AstNodeFTask* taskp = VN_CAST(varp->backp(), NodeFTask)) { - if (const char* reason = cannotSplitTaskReason(taskp)) return reason; + if (const AstNodeFTask* const taskp = VN_CAST(varp->backp(), NodeFTask)) { + if (const char* const reason = cannotSplitTaskReason(taskp)) return reason; + } + if (const char* const reason = cannotSplitVarTypeReason(varp->varType())) { + return reason; + } + if (const char* const reason = cannotSplitVarDirectionReason(varp->direction())) { + return reason; } - if (const char* reason = cannotSplitVarTypeReason(varp->varType())) return reason; - if (const char* reason = cannotSplitVarDirectionReason(varp->direction())) return reason; if (varp->isSigPublic()) return "it is public"; if (varp->isUsedLoopIdx()) return "it is used as a loop variable"; return nullptr; @@ -185,7 +189,7 @@ struct SplitVarImpl { stmtp->unlinkFrBack(); // Insert begin-end because temp value may be inserted to this block later. const std::string name = "__VsplitVarBlk" + cvtToStr(modp->varNumGetInc()); - ap->addStmtp(new AstBegin(ap->fileline(), name, stmtp)); + ap->addStmtp(new AstBegin{ap->fileline(), name, stmtp}); } } @@ -193,28 +197,34 @@ struct SplitVarImpl { if (initp->isJustOneBodyStmt() && initp->bodysp() == stmtp) { stmtp->unlinkFrBack(); // Insert begin-end because temp value may be inserted to this block later. + FileLine* const fl = initp->fileline(); const std::string name = "__VsplitVarBlk" + cvtToStr(modp->varNumGetInc()); - initp->replaceWith( - new AstInitial(initp->fileline(), new AstBegin(initp->fileline(), name, stmtp))); + initp->replaceWith(new AstInitial{fl, new AstBegin{fl, name, stmtp}}); VL_DO_DANGLING(initp->deleteTree(), initp); } } static void insertBeginIfNecessary(AstNodeStmt* stmtp, AstNodeModule* modp) { - AstNode* backp = stmtp->backp(); - if (AstAlways* ap = VN_CAST(backp, Always)) { + AstNode* const backp = stmtp->backp(); + if (AstAlways* const ap = VN_CAST(backp, Always)) { insertBeginCore(ap, stmtp, modp); - } else if (AstAlwaysPublic* ap = VN_CAST(backp, AlwaysPublic)) { + } else if (AstAlwaysPublic* const ap = VN_CAST(backp, AlwaysPublic)) { insertBeginCore(ap, stmtp, modp); - } else if (AstInitial* ap = VN_CAST(backp, Initial)) { + } else if (AstInitial* const ap = VN_CAST(backp, Initial)) { insertBeginCore(ap, stmtp, modp); } } }; // SplitVarImpl -const char* const SplitVarImpl::notSplitMsg - = " has split_var metacomment but will not be split because "; +//###################################################################### +// Utilities required in wharious placs + +static void warnNoSplit(const AstVar* varp, const AstNode* wherep, const char* reasonp) { + wherep->v3warn(SPLITVAR, varp->prettyNameQ() + << " has split_var metacomment but will not be split because " + << reasonp << ".\n"); +} //###################################################################### // Split Unpacked Variables @@ -287,12 +297,11 @@ class UnpackRefMap final { public: using MapType = std::map, AstNodeComparator>; using MapIt = MapType::iterator; - using SetIt = MapType::value_type::second_type::iterator; private: MapType m_map; bool addCore(AstVarRef* refp, const UnpackRef& ref) { - AstVar* varp = refp->varp(); + AstVar* const varp = refp->varp(); UASSERT_OBJ(varp->attrSplitVar(), varp, " no split_var metacomment"); const MapIt it = m_map.find(varp); if (it == m_map.end()) return false; // Not registered @@ -358,23 +367,23 @@ public: v.iterate(nodep); } void visit(AstNVisitor* visitor) { - for (const auto& varp : m_vars) visitor->iterate(varp); - for (auto it = m_sels.begin(), it_end = m_sels.end(); it != it_end; ++it) { + for (AstVar* const varp : m_vars) visitor->iterate(varp); + for (AstSel* const selp : m_sels) { // If m_refs includes VarRef from ArraySel, remove it // because the VarRef would not be visited in SplitPackedVarVisitor::visit(AstSel*). - if (AstVarRef* refp = VN_CAST((*it)->fromp(), VarRef)) { + if (AstVarRef* const refp = VN_CAST(selp->fromp(), VarRef)) { m_refs.erase(refp); - } else if (AstVarRef* refp = VN_CAST((*it)->lsbp(), VarRef)) { + } else if (AstVarRef* const refp = VN_CAST(selp->lsbp(), VarRef)) { m_refs.erase(refp); - } else if (AstVarRef* refp = VN_CAST((*it)->widthp(), VarRef)) { + } else if (AstVarRef* const refp = VN_CAST(selp->widthp(), VarRef)) { m_refs.erase(refp); } - UASSERT_OBJ(reinterpret_cast((*it)->op1p()) != 1, *it, "stale"); - visitor->iterate(*it); + UASSERT_OBJ(reinterpret_cast(selp->op1p()) != 1, selp, "stale"); + visitor->iterate(selp); } - for (auto it = m_refs.begin(), it_end = m_refs.end(); it != it_end; ++it) { - UASSERT_OBJ(reinterpret_cast((*it)->op1p()) != 1, *it, "stale"); - visitor->iterate(*it); + for (AstVarRef* const vrefp : m_refs) { + UASSERT_OBJ(reinterpret_cast(vrefp->op1p()) != 1, vrefp, "stale"); + visitor->iterate(vrefp); } } }; @@ -394,13 +403,14 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { SplitVarRefsMap m_refsForPackedSplit; static AstVarRef* isTargetVref(AstNode* nodep) { - if (AstVarRef* refp = VN_CAST(nodep, VarRef)) { + if (AstVarRef* const refp = VN_CAST(nodep, VarRef)) { if (refp->varp()->attrSplitVar()) return refp; } return nullptr; } - static int outerMostSizeOfUnpackedArray(AstVar* nodep) { - AstUnpackArrayDType* dtypep = VN_CAST(nodep->dtypep()->skipRefp(), UnpackArrayDType); + static int outerMostSizeOfUnpackedArray(const AstVar* nodep) { + const AstUnpackArrayDType* const dtypep + = VN_CAST_CONST(nodep->dtypep()->skipRefp(), UnpackArrayDType); UASSERT_OBJ(dtypep, nodep, "Must be unapcked array"); return dtypep->elementsConst(); } @@ -425,13 +435,13 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { AstNVisitor::pushDeletep(nodep); } AstVar* newVar(FileLine* fl, AstVarType type, const std::string& name, AstNodeDType* dtp) { - AstVar* varp = new AstVar(fl, type, name, dtp); + AstVar* const varp = new AstVar{fl, type, name, dtp}; UASSERT_OBJ(m_modp, varp, "Must not nullptr"); m_refsForPackedSplit[m_modp].add(varp); return varp; } AstVarRef* newVarRef(FileLine* fl, AstVar* varp, const VAccess& access) { - AstVarRef* refp = new AstVarRef(fl, varp, access); + AstVarRef* const refp = new AstVarRef{fl, varp, access}; UASSERT_OBJ(m_modp, refp, "Must not nullptr"); m_refsForPackedSplit[m_modp].add(refp); return refp; @@ -473,7 +483,7 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { VL_RESTORER(m_contextp); { m_contextp = nodep; - AstNodeFTask* ftaskp = nodep->taskp(); + AstNodeFTask* const ftaskp = nodep->taskp(); UASSERT_OBJ(ftaskp, nodep, "Unlinked"); // Iterate arguments of a function/task. for (AstNode *argp = nodep->pinsp(), *paramp = ftaskp->stmtsp(); argp; @@ -495,12 +505,9 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { m_foundTargetVar.clear(); iterate(argp); if (reason) { - for (VarSet::iterator it = m_foundTargetVar.begin(), - it_end = m_foundTargetVar.end(); - it != it_end; ++it) { - argp->v3warn(SPLITVAR, (*it)->prettyNameQ() - << notSplitMsg << reason << ".\n"); - m_refs.remove(*it); + for (AstVar* const varp : m_foundTargetVar) { + warnNoSplit(varp, argp, reason); + m_refs.remove(varp); } } m_foundTargetVar.clear(); @@ -509,15 +516,14 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { } virtual void visit(AstPin* nodep) override { UINFO(5, nodep->modVarp()->prettyNameQ() << " pin \n"); - AstNode* exprp = nodep->exprp(); + AstNode* const exprp = nodep->exprp(); if (!exprp) return; // Not connected pin m_foundTargetVar.clear(); iterate(exprp); if (const char* reason = cannotSplitConnectedPortReason(nodep)) { - for (VarSet::iterator it = m_foundTargetVar.begin(), it_end = m_foundTargetVar.end(); - it != it_end; ++it) { - nodep->v3warn(SPLITVAR, (*it)->prettyNameQ() << notSplitMsg << reason << ".\n"); - m_refs.remove(*it); + for (AstVar* const varp : m_foundTargetVar) { + warnNoSplit(varp, nodep, reason); + m_refs.remove(varp); } m_foundTargetVar.clear(); } @@ -550,22 +556,18 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { iterateChildren(nodep); } virtual void visit(AstArraySel* nodep) override { - if (AstVarRef* refp = isTargetVref(nodep->fromp())) { - AstConst* indexp = VN_CAST(nodep->bitp(), Const); + if (AstVarRef* const refp = isTargetVref(nodep->fromp())) { + AstConst* const indexp = VN_CAST(nodep->bitp(), Const); if (indexp) { // OK UINFO(4, "add " << nodep << " for " << refp->varp()->prettyName() << "\n"); if (indexp->toSInt() < outerMostSizeOfUnpackedArray(refp->varp())) { m_refs.tryAdd(m_contextp, refp, nodep, indexp->toSInt(), m_inFTask); } else { - nodep->bitp()->v3warn(SPLITVAR, refp->prettyNameQ() - << notSplitMsg - << "index is out of range.\n"); + warnNoSplit(refp->varp(), nodep->bitp(), "index is out of range"); m_refs.remove(refp->varp()); } } else { - nodep->bitp()->v3warn(SPLITVAR, refp->prettyNameQ() - << notSplitMsg - << "index cannot be determined statically.\n"); + warnNoSplit(refp->varp(), nodep->bitp(), "index cannot be determined statically"); m_refs.remove(refp->varp()); iterate(nodep->bitp()); } @@ -574,8 +576,8 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { } } virtual void visit(AstSliceSel* nodep) override { - if (AstVarRef* refp = isTargetVref(nodep->fromp())) { - AstUnpackArrayDType* dtypep + if (AstVarRef* const refp = isTargetVref(nodep->fromp())) { + const AstUnpackArrayDType* const dtypep = VN_CAST(refp->varp()->dtypep()->skipRefp(), UnpackArrayDType); // declRange() of AstSliceSel is shifted by dtypep->declRange().lo() in V3WidthSel.cpp // restore the original decl range here. @@ -592,7 +594,7 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { } } static AstNode* toInsertPoint(AstNode* insertp) { - if (AstNodeStmt* stmtp = VN_CAST(insertp, NodeStmt)) { + if (AstNodeStmt* const stmtp = VN_CAST(insertp, NodeStmt)) { if (!stmtp->isStatement()) insertp = stmtp->backp(); } return insertp; @@ -600,15 +602,16 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { AstVarRef* createTempVar(AstNode* context, AstNode* nodep, AstUnpackArrayDType* dtypep, const std::string& name_prefix, std::vector& vars, int start_idx, bool lvalue, bool ftask) { + FileLine* const fl = nodep->fileline(); const std::string name = "__VsplitVar" + cvtToStr(m_modp->varNumGetInc()) + "__" + name_prefix; - AstNodeAssign* assignp = VN_CAST(context, NodeAssign); + AstNodeAssign* const assignp = VN_CAST(context, NodeAssign); if (assignp) { // "always_comb a = b;" to "always_comb begin a = b; end" so that local // variable can be added. insertBeginIfNecessary(assignp, m_modp); } - AstVar* varp = newVar(nodep->fileline(), AstVarType::VAR, name, dtypep); + AstVar* const varp = newVar(fl, AstVarType::VAR, name, dtypep); // Variable will be registered in the caller side. UINFO(3, varp->prettyNameQ() << " is created lsb:" << dtypep->lo() << " msb:" << dtypep->hi() << "\n"); @@ -617,18 +620,17 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { = (context && VN_IS(context, NodeFTaskRef)) || (assignp && VN_IS(assignp, Assign)); for (int i = 0; i < dtypep->elementsConst(); ++i) { - AstNode* lhsp = newVarRef(nodep->fileline(), vars.at(start_idx + i), - lvalue ? VAccess::WRITE : VAccess::READ); - AstNode* rhsp = new AstArraySel( - nodep->fileline(), - newVarRef(nodep->fileline(), varp, !lvalue ? VAccess::WRITE : VAccess::READ), i); + AstNode* lhsp + = newVarRef(fl, vars.at(start_idx + i), lvalue ? VAccess::WRITE : VAccess::READ); + AstNode* rhsp = new AstArraySel{ + fl, newVarRef(fl, varp, !lvalue ? VAccess::WRITE : VAccess::READ), i}; AstNode* refp = lhsp; UINFO(9, "Creating assign idx:" << i << " + " << start_idx << "\n"); if (!lvalue) std::swap(lhsp, rhsp); AstNode* newassignp; if (use_simple_assign) { - AstNode* insertp = toInsertPoint(context); - newassignp = new AstAssign(nodep->fileline(), lhsp, rhsp); + AstNode* const insertp = toInsertPoint(context); + newassignp = new AstAssign{fl, lhsp, rhsp}; if (lvalue) { // If varp is LHS, this assignment must appear after the original // assignment(context). @@ -638,28 +640,28 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { insertp->addHereThisAsNext(newassignp); } } else { - newassignp = new AstAssignW(nodep->fileline(), lhsp, rhsp); + newassignp = new AstAssignW{fl, lhsp, rhsp}; // Continuous assignment must be in module context. varp->addNextHere(newassignp); } UASSERT_OBJ(!m_contextp, m_contextp, "must be null"); setContextAndIterate(newassignp, refp); } - return newVarRef(nodep->fileline(), varp, lvalue ? VAccess::WRITE : VAccess::READ); + return newVarRef(fl, varp, lvalue ? VAccess::WRITE : VAccess::READ); } void connectPort(AstVar* varp, std::vector& vars, AstNode* insertp) { UASSERT_OBJ(varp->isIO(), varp, "must be port"); insertp = insertp ? toInsertPoint(insertp) : nullptr; const bool lvalue = varp->direction().isWritable(); + FileLine* const fl = varp->fileline(); for (size_t i = 0; i < vars.size(); ++i) { - AstNode* nodes[] = { - new AstArraySel( - varp->fileline(), - newVarRef(varp->fileline(), varp, lvalue ? VAccess::WRITE : VAccess::READ), i), - newVarRef(varp->fileline(), vars.at(i), !lvalue ? VAccess::WRITE : VAccess::READ)}; - AstNode* lhsp = nodes[lvalue ? 0 : 1]; - AstNode* rhsp = nodes[lvalue ? 1 : 0]; - AstNodeAssign* assignp = newAssign(varp->fileline(), lhsp, rhsp, varp); + AstNode* const nodes[] = { + new AstArraySel{fl, newVarRef(fl, varp, lvalue ? VAccess::WRITE : VAccess::READ), + static_cast(i)}, + newVarRef(fl, vars.at(i), !lvalue ? VAccess::WRITE : VAccess::READ)}; + AstNode* const lhsp = nodes[lvalue ? 0 : 1]; + AstNode* const rhsp = nodes[lvalue ? 1 : 0]; + AstNodeAssign* const assignp = newAssign(fl, lhsp, rhsp, varp); if (insertp) { if (lvalue) { // Just after writing to the temporary variable insertp->addNextHere(assignp); @@ -675,14 +677,15 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { } size_t collapse(UnpackRefMap& refs) { size_t numSplit = 0; - for (UnpackRefMap::MapIt it = refs.begin(), it_end = refs.end(); it != it_end; ++it) { - UINFO(3, "In module " << m_modp->name() << " var " << it->first->prettyNameQ() - << " which has " << it->second.size() + for (const auto& pair : refs) { + UINFO(3, "In module " << m_modp->name() << " var " << pair.first->prettyNameQ() + << " which has " << pair.second.size() << " refs will be split.\n"); - AstVar* varp = it->first; + AstVar* const varp = pair.first; AstNode* insertp = varp; - AstUnpackArrayDType* dtypep = VN_CAST(varp->dtypep()->skipRefp(), UnpackArrayDType); - AstNodeDType* subTypep = dtypep->subDTypep(); + const AstUnpackArrayDType* const dtypep + = VN_CAST(varp->dtypep()->skipRefp(), UnpackArrayDType); + AstNodeDType* const subTypep = dtypep->subDTypep(); const bool needNext = VN_IS(subTypep, UnpackArrayDType); // Still unpacked array. std::vector vars; // Add the split variables @@ -701,39 +704,38 @@ class SplitUnpackedVarVisitor final : public AstNVisitor, public SplitVarImpl { vars.push_back(newp); setContextAndIterate(nullptr, newp); } - for (UnpackRefMap::SetIt sit = it->second.begin(), sit_end = it->second.end(); - sit != sit_end; ++sit) { + for (const UnpackRef& ref : pair.second) { AstNode* newp = nullptr; - if (sit->isSingleRef()) { - newp = newVarRef(sit->nodep()->fileline(), vars.at(sit->index()), - sit->access()); + if (ref.isSingleRef()) { + newp = newVarRef(ref.nodep()->fileline(), vars.at(ref.index()), ref.access()); } else { - AstVarRef* refp = VN_CAST(sit->nodep(), VarRef); + AstVarRef* refp = VN_CAST(ref.nodep(), VarRef); AstUnpackArrayDType* adtypep; int lsb = 0; if (refp) { adtypep = VN_CAST(refp->dtypep()->skipRefp(), UnpackArrayDType); } else { - AstSliceSel* selp = VN_CAST(sit->nodep(), SliceSel); - UASSERT_OBJ(selp, sit->nodep(), "Unexpected op is registered"); + AstSliceSel* selp = VN_CAST(ref.nodep(), SliceSel); + UASSERT_OBJ(selp, ref.nodep(), "Unexpected op is registered"); refp = VN_CAST(selp->fromp(), VarRef); UASSERT_OBJ(refp, selp, "Unexpected op is registered"); adtypep = VN_CAST(selp->dtypep()->skipRefp(), UnpackArrayDType); lsb = adtypep->lo(); } - AstVarRef* newrefp = createTempVar(sit->context(), refp, adtypep, varp->name(), - vars, lsb, refp->access(), sit->ftask()); + AstVarRef* const newrefp + = createTempVar(ref.context(), refp, adtypep, varp->name(), vars, lsb, + refp->access(), ref.ftask()); newp = newrefp; refp->varp()->addNextHere(newrefp->varp()); UINFO(3, "Create " << newrefp->varp()->prettyNameQ() << " for " << refp << "\n"); } - sit->nodep()->replaceWith(newp); - pushDeletep(sit->nodep()); - setContextAndIterate(sit->context(), newp->backp()); + ref.nodep()->replaceWith(newp); + pushDeletep(ref.nodep()); + setContextAndIterate(ref.context(), newp->backp()); // AstAssign is used. So assignment is necessary for each reference. if (varp->isIO() && (varp->isFuncLocal() || varp->isFuncReturn())) - connectPort(varp, vars, sit->context()); + connectPort(varp, vars, ref.context()); } if (varp->isIO()) { // AssignW will be created, so just once @@ -850,7 +852,9 @@ public: // If this is AstVarRef and referred in the sensitivity list of always@, // return the sensitivity item AstSenItem* backSenItemp() const { - if (AstVarRef* refp = VN_CAST(m_nodep, VarRef)) return VN_CAST(refp->backp(), SenItem); + if (AstVarRef* const refp = VN_CAST(m_nodep, VarRef)) { + return VN_CAST(refp->backp(), SenItem); + } return nullptr; } }; @@ -872,9 +876,7 @@ class PackedVarRef final { for (size_t i = 0; i < refs.size(); ++i) { nodes.emplace(refs[i].nodep(), i); } std::vector vect; vect.reserve(nodes.size()); - for (auto it = nodes.cbegin(), it_end = nodes.cend(); it != it_end; ++it) { - vect.push_back(refs[it->second]); - } + for (const auto& pair : nodes) vect.push_back(refs[pair.second]); refs.swap(vect); } @@ -910,16 +912,16 @@ public: std::vector plan; std::vector> points; // points.reserve(m_lhs.size() * 2 + 2); // 2 points will be added per one PackedVarRefEntry - for (const_iterator it = m_lhs.begin(), itend = m_lhs.end(); it != itend; ++it) { - points.emplace_back(std::make_pair(it->lsb(), false)); // Start of a region - points.emplace_back(std::make_pair(it->msb() + 1, true)); // End of a region + for (const PackedVarRefEntry& ref : m_lhs) { + points.emplace_back(std::make_pair(ref.lsb(), false)); // Start of a region + points.emplace_back(std::make_pair(ref.msb() + 1, true)); // End of a region } if (skipUnused && !m_rhs.empty()) { // Range to be read must be kept, so add points here int lsb = m_basicp->hi() + 1; int msb = m_basicp->lo() - 1; - for (size_t i = 0; i < m_rhs.size(); ++i) { - lsb = std::min(lsb, m_rhs[i].lsb()); - msb = std::max(msb, m_rhs[i].msb()); + for (const PackedVarRefEntry& ref : m_rhs) { + lsb = std::min(lsb, ref.lsb()); + msb = std::max(msb, ref.msb()); } UASSERT_OBJ(lsb <= msb, m_basicp, "lsb:" << lsb << " msb:" << msb << " are wrong"); points.emplace_back(std::make_pair(lsb, false)); @@ -960,8 +962,8 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { } virtual void visit(AstVar* nodep) override { if (!nodep->attrSplitVar()) return; // Nothing to do - if (const char* reason = cannotSplitReason(nodep, true)) { - nodep->v3warn(SPLITVAR, nodep->prettyNameQ() << notSplitMsg << reason); + if (const char* const reason = cannotSplitReason(nodep, true)) { + warnNoSplit(nodep, nodep, reason); nodep->attrSplitVar(false); } else { // Finally find a good candidate const bool inserted = m_refs.insert(std::make_pair(nodep, PackedVarRef(nodep))).second; @@ -969,7 +971,7 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { } } virtual void visit(AstVarRef* nodep) override { - AstVar* varp = nodep->varp(); + AstVar* const varp = nodep->varp(); visit(varp); const auto refit = m_refs.find(varp); if (refit == m_refs.end()) return; // variable without split_var metacomment @@ -983,13 +985,13 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { << " Entire bit of [" << basicp->lo() << "+:" << varp->width() << "] \n"); } virtual void visit(AstSel* nodep) override { - AstVarRef* vrefp = VN_CAST(nodep->fromp(), VarRef); + AstVarRef* const vrefp = VN_CAST(nodep->fromp(), VarRef); if (!vrefp) { iterateChildren(nodep); return; } - AstVar* varp = vrefp->varp(); + AstVar* const varp = vrefp->varp(); const auto refit = m_refs.find(varp); if (refit == m_refs.end()) { iterateChildren(nodep); @@ -1009,9 +1011,7 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { << " [" << consts[0]->toSInt() << ":+" << consts[1]->toSInt() << "] lsb:" << refit->second.basicp()->lo() << "\n"); } else { - nodep->v3warn(SPLITVAR, vrefp->prettyNameQ() - << notSplitMsg - << "its bit range cannot be determined statically."); + warnNoSplit(vrefp->varp(), nodep, "its bit range cannot be determined statically"); if (!consts[0]) { UINFO(4, "LSB " << nodep->lsbp() << " is expected to be constant, but not\n"); } @@ -1028,7 +1028,8 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { // Extract necessary bit range from a newly created variable to meet ref static AstNode* extractBits(const PackedVarRefEntry& ref, const SplitNewVar& var, const VAccess access) { - AstVarRef* refp = new AstVarRef(ref.nodep()->fileline(), var.varp(), access); + FileLine* const fl = ref.nodep()->fileline(); + AstVarRef* const refp = new AstVarRef{fl, var.varp(), access}; if (ref.lsb() <= var.lsb() && var.msb() <= ref.msb()) { // Use the entire bits return refp; } else { // Use slice @@ -1038,27 +1039,25 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { UINFO(4, var.varp()->prettyNameQ() << "[" << msb << ":" << lsb << "] used for " << ref.nodep()->prettyNameQ() << '\n'); // LSB of varp is always 0. "lsb - var.lsb()" means this. see also SplitNewVar - AstSel* selp = new AstSel(ref.nodep()->fileline(), refp, lsb - var.lsb(), bitwidth); - return selp; + return new AstSel{fl, refp, lsb - var.lsb(), bitwidth}; } } static void connectPortAndVar(const std::vector& vars, AstVar* portp, AstNode* insertp) { for (; insertp; insertp = insertp->backp()) { - if (AstNodeStmt* stmtp = VN_CAST(insertp, NodeStmt)) { + if (const AstNodeStmt* const stmtp = VN_CAST(insertp, NodeStmt)) { if (stmtp->isStatement()) break; } } const bool in = portp->isReadOnly(); - for (size_t i = 0; i < vars.size(); ++i) { - AstNode* rhsp = new AstSel( - portp->fileline(), - new AstVarRef(portp->fileline(), portp, !in ? VAccess::WRITE : VAccess::READ), - vars[i].lsb(), vars[i].bitwidth()); - AstNode* lhsp = new AstVarRef(portp->fileline(), vars[i].varp(), - in ? VAccess::WRITE : VAccess::READ); + FileLine* const fl = portp->fileline(); + for (const SplitNewVar& var : vars) { + AstNode* rhsp + = new AstSel{fl, new AstVarRef{fl, portp, !in ? VAccess::WRITE : VAccess::READ}, + var.lsb(), var.bitwidth()}; + AstNode* lhsp = new AstVarRef{fl, var.varp(), in ? VAccess::WRITE : VAccess::READ}; if (!in) std::swap(lhsp, rhsp); - AstNodeAssign* assignp = newAssign(portp->fileline(), lhsp, rhsp, portp); + AstNodeAssign* const assignp = newAssign(fl, lhsp, rhsp, portp); if (insertp) { if (in) { insertp->addHereThisAsNext(assignp); @@ -1066,15 +1065,14 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { insertp->addNextHere(assignp); } } else { - vars[i].varp()->addNextHere(assignp); + var.varp()->addNextHere(assignp); } } } void createVars(AstVar* varp, const AstBasicDType* basicp, std::vector& vars) { - for (size_t i = 0; i < vars.size(); ++i) { - SplitNewVar* newvarp = &vars[i]; - int left = newvarp->msb(); - int right = newvarp->lsb(); + for (SplitNewVar& newvar : vars) { + int left = newvar.msb(); + int right = newvar.lsb(); if (basicp->littleEndian()) std::swap(left, right); const std::string name = (left == right) @@ -1085,93 +1083,93 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { AstBasicDType* dtypep; switch (basicp->keyword()) { case AstBasicDTypeKwd::BIT: - dtypep = new AstBasicDType(varp->subDTypep()->fileline(), VFlagBitPacked(), - newvarp->bitwidth()); + dtypep = new AstBasicDType{varp->subDTypep()->fileline(), VFlagBitPacked(), + newvar.bitwidth()}; break; case AstBasicDTypeKwd::LOGIC: - dtypep = new AstBasicDType(varp->subDTypep()->fileline(), VFlagLogicPacked(), - newvarp->bitwidth()); + dtypep = new AstBasicDType{varp->subDTypep()->fileline(), VFlagLogicPacked(), + newvar.bitwidth()}; break; default: UASSERT_OBJ(false, basicp, "Only bit and logic are allowed"); } - dtypep->rangep(new AstRange{varp->fileline(), VNumRange{newvarp->msb(), newvarp->lsb(), - basicp->littleEndian()}}); - newvarp->varp(new AstVar(varp->fileline(), AstVarType::VAR, name, dtypep)); - newvarp->varp()->propagateAttrFrom(varp); - newvarp->varp()->funcLocal(varp->isFuncLocal() || varp->isFuncReturn()); + dtypep->rangep(new AstRange{ + varp->fileline(), VNumRange{newvar.msb(), newvar.lsb(), basicp->littleEndian()}}); + newvar.varp(new AstVar{varp->fileline(), AstVarType::VAR, name, dtypep}); + newvar.varp()->propagateAttrFrom(varp); + newvar.varp()->funcLocal(varp->isFuncLocal() || varp->isFuncReturn()); // Enable this line to trace split variable directly: - // newvarp->varp()->trace(varp->isTrace()); + // newvar.varp()->trace(varp->isTrace()); m_netp->typeTablep()->addTypesp(dtypep); - varp->addNextHere(newvarp->varp()); - UINFO(4, newvarp->varp()->prettyNameQ() + varp->addNextHere(newvar.varp()); + UINFO(4, newvar.varp()->prettyNameQ() << " is added for " << varp->prettyNameQ() << '\n'); } } static void updateReferences(AstVar* varp, PackedVarRef& ref, const std::vector& vars) { - for (int lvalue = 0; lvalue <= 1; ++lvalue) { // Refer the new split variables + for (const bool lvalue : {false, true}) { // Refer the new split variables std::vector& refs = lvalue ? ref.lhs() : ref.rhs(); - for (PackedVarRef::iterator refit = refs.begin(), refitend = refs.end(); - refit != refitend; ++refit) { - auto varit = std::upper_bound(vars.begin(), vars.end(), refit->lsb(), - SplitNewVar::Match()); - UASSERT_OBJ(varit != vars.end(), refit->nodep(), "Not found"); - UASSERT(!(varit->msb() < refit->lsb() || refit->msb() < varit->lsb()), + for (PackedVarRefEntry& ref : refs) { + auto varit + = std::upper_bound(vars.begin(), vars.end(), ref.lsb(), SplitNewVar::Match()); + UASSERT_OBJ(varit != vars.end(), ref.nodep(), "Not found"); + UASSERT(!(varit->msb() < ref.lsb() || ref.msb() < varit->lsb()), "wrong search result"); AstNode* prevp; bool inSentitivityList = false; - if (AstSenItem* senitemp = refit->backSenItemp()) { - AstNode* oldsenrefp = senitemp->sensp(); + if (AstSenItem* const senitemp = ref.backSenItemp()) { + AstNode* const oldsenrefp = senitemp->sensp(); oldsenrefp->replaceWith( - new AstVarRef(senitemp->fileline(), varit->varp(), VAccess::READ)); + new AstVarRef{senitemp->fileline(), varit->varp(), VAccess::READ}); VL_DO_DANGLING(oldsenrefp->deleteTree(), oldsenrefp); prevp = senitemp; inSentitivityList = true; } else { - prevp = extractBits(*refit, *varit, lvalue ? VAccess::WRITE : VAccess::READ); + prevp = extractBits(ref, *varit, lvalue ? VAccess::WRITE : VAccess::READ); } - for (int residue = refit->msb() - varit->msb(); residue > 0; + for (int residue = ref.msb() - varit->msb(); residue > 0; residue -= varit->bitwidth()) { ++varit; - UASSERT_OBJ(varit != vars.end(), refit->nodep(), "not enough split variables"); - if (AstSenItem* senitemp = VN_CAST(prevp, SenItem)) { - prevp = new AstSenItem( + UASSERT_OBJ(varit != vars.end(), ref.nodep(), "not enough split variables"); + if (AstSenItem* const senitemp = VN_CAST(prevp, SenItem)) { + prevp = new AstSenItem{ senitemp->fileline(), senitemp->edgeType(), - new AstVarRef(senitemp->fileline(), varit->varp(), VAccess::READ)); + new AstVarRef{senitemp->fileline(), varit->varp(), VAccess::READ}}; senitemp->addNextHere(prevp); } else { - AstNode* bitsp - = extractBits(*refit, *varit, lvalue ? VAccess::WRITE : VAccess::READ); - prevp = new AstConcat(refit->nodep()->fileline(), bitsp, prevp); + AstNode* const bitsp + = extractBits(ref, *varit, lvalue ? VAccess::WRITE : VAccess::READ); + prevp = new AstConcat{ref.nodep()->fileline(), bitsp, prevp}; } } // If varp is an argument of task/func, need to update temporary var // everytime the var is updated. See also another call of connectPortAndVar() in // split() if (varp->isIO() && (varp->isFuncLocal() || varp->isFuncReturn())) - connectPortAndVar(vars, varp, refit->nodep()); - if (!inSentitivityList) refit->replaceNodeWith(prevp); - UASSERT_OBJ(varit->msb() >= refit->msb(), varit->varp(), "Out of range"); + connectPortAndVar(vars, varp, ref.nodep()); + if (!inSentitivityList) ref.replaceNodeWith(prevp); + UASSERT_OBJ(varit->msb() >= ref.msb(), varit->varp(), "Out of range"); } } } // Do the actual splitting operation void split() { - for (auto it = m_refs.begin(), it_end = m_refs.end(); it != it_end; ++it) { - it->second.dedup(); - AstVar* varp = it->first; + for (auto& pair : m_refs) { + AstVar* const varp = pair.first; + PackedVarRef& ref = pair.second; + ref.dedup(); UINFO(3, "In module " << m_modp->name() << " var " << varp->prettyNameQ() - << " which has " << it->second.lhs().size() << " lhs refs and " - << it->second.rhs().size() << " rhs refs will be split.\n"); + << " which has " << ref.lhs().size() << " lhs refs and " + << ref.rhs().size() << " rhs refs will be split.\n"); std::vector vars - = it->second.splitPlan(!varp->isTrace()); // If traced, all bit must be kept + = ref.splitPlan(!varp->isTrace()); // If traced, all bit must be kept if (vars.empty()) continue; if (vars.size() == 1 && vars.front().bitwidth() == varp->width()) continue; // No split - createVars(varp, it->second.basicp(), vars); // Add the split variables + createVars(varp, ref.basicp(), vars); // Add the split variables - updateReferences(varp, it->second, vars); + updateReferences(varp, ref, vars); if (varp->isIO()) { // port cannot be deleted // If varp is a port of a module, single AssignW is sufficient @@ -1179,16 +1177,15 @@ class SplitPackedVarVisitor final : public AstNVisitor, public SplitVarImpl { connectPortAndVar(vars, varp, nullptr); } else if (varp->isTrace()) { // Let's reuse the original variable for tracing - AstNode* rhsp = new AstVarRef(vars.front().varp()->fileline(), vars.front().varp(), - VAccess::READ); + AstNode* rhsp = new AstVarRef{vars.front().varp()->fileline(), vars.front().varp(), + VAccess::READ}; + FileLine* const fl = varp->fileline(); for (size_t i = 1; i < vars.size(); ++i) { - rhsp = new AstConcat( - varp->fileline(), - new AstVarRef(varp->fileline(), vars[i].varp(), VAccess::READ), rhsp); + rhsp = new AstConcat{fl, new AstVarRef{fl, vars[i].varp(), VAccess::READ}, + rhsp}; } - varp->addNextHere(newAssign(varp->fileline(), - new AstVarRef(varp->fileline(), varp, VAccess::WRITE), - rhsp, varp)); + varp->addNextHere( + newAssign(fl, new AstVarRef{fl, varp, VAccess::WRITE}, rhsp, varp)); } else { // the original variable is not used anymore. VL_DO_DANGLING(varp->unlinkFrBack()->deleteTree(), varp); } diff --git a/test_regress/t/t_split_var_1_bad.out b/test_regress/t/t_split_var_1_bad.out index 40dab2d0e..6249c2943 100644 --- a/test_regress/t/t_split_var_1_bad.out +++ b/test_regress/t/t_split_var_1_bad.out @@ -35,35 +35,35 @@ : ... In instance t.i_sub3 90 | assign outwires[12] = inwires[13]; | ^~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:17:9: 'should_show_warning0' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic +%Warning-SPLITVAR: t/t_split_var_1_bad.v:17:9: 'should_show_warning0' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic. : ... In instance t 17 | real should_show_warning0 /*verilator split_var*/ ; | ^~~~~~~~~~~~~~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:18:11: 'should_show_warning1' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic +%Warning-SPLITVAR: t/t_split_var_1_bad.v:18:11: 'should_show_warning1' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic. : ... In instance t 18 | string should_show_warning1 /*verilator split_var*/ ; | ^~~~~~~~~~~~~~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:19:11: 'should_show_warning2' has split_var metacomment but will not be split because its bitwidth is 1 +%Warning-SPLITVAR: t/t_split_var_1_bad.v:19:11: 'should_show_warning2' has split_var metacomment but will not be split because its bitwidth is 1. : ... In instance t 19 | wire should_show_warning2 /*verilator split_var*/ ; | ^~~~~~~~~~~~~~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:23:16: 'public_signal' has split_var metacomment but will not be split because it is public +%Warning-SPLITVAR: t/t_split_var_1_bad.v:23:16: 'public_signal' has split_var metacomment but will not be split because it is public. : ... In instance t 23 | logic [1:0] public_signal /*verilator public*/ /*verilator split_var*/ ; | ^~~~~~~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:31:44: 'inout_port' has split_var metacomment but will not be split because it is an inout port +%Warning-SPLITVAR: t/t_split_var_1_bad.v:31:44: 'inout_port' has split_var metacomment but will not be split because it is an inout port. : ... In instance t 31 | function int bad_func(inout logic [3:0] inout_port /*verilator split_var*/ , | ^~~~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:32:42: 'ref_port' has split_var metacomment but will not be split because it is a ref argument +%Warning-SPLITVAR: t/t_split_var_1_bad.v:32:42: 'ref_port' has split_var metacomment but will not be split because it is a ref argument. : ... In instance t 32 | ref logic [7:0] ref_port /*verilator split_var*/ ); | ^~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:37:19: 'loop_idx' has split_var metacomment but will not be split because it is used as a loop variable +%Warning-SPLITVAR: t/t_split_var_1_bad.v:37:19: 'loop_idx' has split_var metacomment but will not be split because it is used as a loop variable. : ... In instance t 37 | logic [7:0] loop_idx /*verilator split_var*/ ; | ^~~~~~~~ -%Warning-SPLITVAR: t/t_split_var_1_bad.v:62:11: 'cannot_split_genvar' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic +%Warning-SPLITVAR: t/t_split_var_1_bad.v:62:11: 'cannot_split_genvar' has split_var metacomment but will not be split because it is not an aggregate type of bit nor logic. : ... In instance t.i_sub1 62 | genvar cannot_split_genvar /*verilator split_var*/ ; | ^~~~~~~~~~~~~~~~~~~