From 608379d3570d85c73eb9998e8b1c017509cd445e Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Thu, 18 Jun 2026 20:24:27 +0100 Subject: [PATCH] Review fixes --- include/verilated.cpp | 108 ------------------------------- include/verilated_funcs.h | 48 ++++++++++---- src/V3AstInlines.h | 8 +-- src/V3AstNodeExpr.h | 32 ++++----- src/V3AstNodes.cpp | 20 ++++++ src/V3Case.cpp | 31 ++++++--- src/V3Clean.cpp | 4 +- src/V3Const.cpp | 11 ++-- src/V3Dfg.cpp | 23 ++++--- src/V3Dfg.h | 7 +- src/V3DfgCse.cpp | 4 +- src/V3DfgDfgToAst.cpp | 8 +-- src/V3DfgPeephole.cpp | 10 +++ src/V3DfgPeepholePatterns.h | 1 + src/V3DfgSynthesize.cpp | 11 ++-- src/V3DfgVertices.h | 33 +++++----- src/V3EmitCFunc.h | 23 +------ src/V3Gate.cpp | 5 ++ src/V3Premit.cpp | 8 ++- test_regress/t/t_case_decoder.py | 2 +- test_regress/t/t_case_decoder.v | 17 +++++ test_regress/t/t_dfg_peephole.v | 46 +++++++------ 22 files changed, 214 insertions(+), 246 deletions(-) diff --git a/include/verilated.cpp b/include/verilated.cpp index 718ba7846..fcfbab58a 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -2876,114 +2876,6 @@ void VL_WRITEMEM_N(bool hex, // Hex format, else binary } } -//=========================================================================== -// Decoder functions - -// I input -IData VL_DECODER_II(IData index, WDataInP matchp, WDataInP valuep) VL_PURE { - size_t i = 0; - while (true) { - const IData mask = matchp[i * 2]; - const IData bits = matchp[i * 2 + 1]; - if ((mask & index) == bits) break; - ++i; - } - return valuep[i]; -} -QData VL_DECODER_QI(IData index, WDataInP matchp, WDataInP valuep) VL_PURE { - size_t i = 0; - while (true) { - const IData mask = matchp[i * 2]; - const IData bits = matchp[i * 2 + 1]; - if ((mask & index) == bits) break; - ++i; - } - return VL_SET_QW(valuep + 2 * i); -} -WDataOutP VL_DECODER_WI(int owords, WDataOutP owp, IData index, WDataInP matchp, - WDataInP valuep) VL_MT_SAFE { - size_t i = 0; - while (true) { - const IData mask = matchp[i * 2]; - const IData bits = matchp[i * 2 + 1]; - if ((mask & index) == bits) break; - ++i; - } - return VL_MEMCPY_W(owp, valuep + owords * i, owords); -} - -// Q input -IData VL_DECODER_IQ(QData index, WDataInP matchp, WDataInP valuep) VL_PURE { - size_t i = 0; - while (true) { - const QData mask = VL_SET_QW(matchp + i * 4); - const QData bits = VL_SET_QW(matchp + i * 4 + 2); - if ((mask & index) == bits) break; - ++i; - } - return valuep[i]; -} -QData VL_DECODER_QQ(QData index, WDataInP matchp, WDataInP valuep) VL_PURE { - size_t i = 0; - while (true) { - const QData mask = VL_SET_QW(matchp + i * 4); - const QData bits = VL_SET_QW(matchp + i * 4 + 2); - if ((mask & index) == bits) break; - ++i; - } - return VL_SET_QW(valuep + 2 * i); -} -WDataOutP VL_DECODER_WQ(int owords, WDataOutP owp, QData index, WDataInP matchp, - WDataInP valuep) VL_MT_SAFE { - size_t i = 0; - while (true) { - const QData mask = VL_SET_QW(matchp + i * 4); - const QData bits = VL_SET_QW(matchp + i * 4 + 2); - if ((mask & index) == bits) break; - ++i; - } - return VL_MEMCPY_W(owp, valuep + owords * i, owords); -} - -// W input -IData VL_DECODER_IW(int iwords, WDataInP indexp, WDataInP matchp, WDataInP valuep) VL_PURE { - size_t i = 0; - while (true) { - const WDataInP maskp = matchp + i * iwords * 2; - const WDataInP bitsp = matchp + i * iwords * 2 + iwords; - EData diff = 0; - for (size_t j = 0; j < iwords; ++j) diff |= (maskp[j] & indexp[j]) ^ bitsp[j]; - if (!diff) break; - ++i; - } - return valuep[i]; -} -QData VL_DECODER_QW(int iwords, WDataInP indexp, WDataInP matchp, WDataInP valuep) VL_PURE { - size_t i = 0; - while (true) { - const WDataInP maskp = matchp + i * iwords * 2; - const WDataInP bitsp = matchp + i * iwords * 2 + iwords; - EData diff = 0; - for (size_t j = 0; j < iwords; ++j) diff |= (maskp[j] & indexp[j]) ^ bitsp[j]; - if (!diff) break; - ++i; - } - return VL_SET_QW(valuep + 2 * i); -} -WDataOutP VL_DECODER_WW(int owords, WDataOutP owp, int iwords, WDataInP indexp, WDataInP matchp, - WDataInP valuep) VL_MT_SAFE { - size_t i = 0; - while (true) { - const WDataInP maskp = matchp + i * iwords * 2; - const WDataInP bitsp = matchp + i * iwords * 2 + iwords; - EData diff = 0; - for (size_t j = 0; j < iwords; ++j) diff |= (maskp[j] & indexp[j]) ^ bitsp[j]; - if (!diff) break; - ++i; - } - return VL_MEMCPY_W(owp, valuep + owords * i, owords); -} - //=========================================================================== // Timescale conversion diff --git a/include/verilated_funcs.h b/include/verilated_funcs.h index acd434cb5..da247b63f 100644 --- a/include/verilated_funcs.h +++ b/include/verilated_funcs.h @@ -269,20 +269,40 @@ static inline VlQueue VL_CVT_UNPACK_TO_Q(const VlUnpacked& q) VL_ return ret; } -IData VL_DECODER_II(IData index, WDataInP matchp, WDataInP valuep) VL_PURE; -QData VL_DECODER_QI(IData index, WDataInP matchp, WDataInP valuep) VL_PURE; -WDataOutP VL_DECODER_WI(int owords, WDataOutP owp, IData index, WDataInP matchp, - WDataInP valuep) VL_MT_SAFE; - -IData VL_DECODER_IQ(QData index, WDataInP matchp, WDataInP valuep) VL_PURE; -QData VL_DECODER_QQ(QData index, WDataInP matchp, WDataInP valuep) VL_PURE; -WDataOutP VL_DECODER_WQ(int owords, WDataOutP owp, QData index, WDataInP matchp, - WDataInP valuep) VL_MT_SAFE; - -IData VL_DECODER_IW(int iwords, WDataInP indexp, WDataInP matchp, WDataInP valuep) VL_PURE; -QData VL_DECODER_QW(int iwords, WDataInP indexp, WDataInP matchp, WDataInP valuep) VL_PURE; -WDataOutP VL_DECODER_WW(int owords, WDataOutP owp, int iwords, WDataInP indexp, WDataInP matchp, - WDataInP valuep) VL_MT_SAFE; +// Masked match functions +static inline IData VL_MATCHMASKED_I(int , IData lhs, WDataInP matchp) VL_PURE { + size_t i = 0; + while (true) { + const IData mask = matchp[i * 2]; + const IData bits = matchp[i * 2 + 1]; + if ((mask & lhs) == bits) break; + ++i; + } + return i; +} +static inline IData VL_MATCHMASKED_Q(int, QData lhs, WDataInP matchp) VL_PURE { + size_t i = 0; + while (true) { + const QData mask = VL_SET_QW(matchp + i * 4); + const QData bits = VL_SET_QW(matchp + i * 4 + 2); + if ((mask & lhs) == bits) break; + ++i; + } + return i; +} +static inline IData VL_MATCHMASKED_W(int lbits, WDataInP lhsp, WDataInP matchp) VL_MT_SAFE { + const int iwords = VL_WORDS_I(lbits); + size_t i = 0; + while (true) { + const WDataInP maskp = matchp + (i * iwords * 2); + const WDataInP bitsp = matchp + (i * iwords * 2 + iwords); + EData diff = 0; + for (int j = 0; j < iwords; ++j) diff |= (maskp[j] & lhsp[j]) ^ bitsp[j]; + if (!diff) break; + ++i; + } + return i; +} // Return double from lhs (numeric) unsigned double VL_ITOR_D_W(int lbits, WDataInP const lwp) VL_PURE; diff --git a/src/V3AstInlines.h b/src/V3AstInlines.h index 938d93e65..6ef16eec6 100644 --- a/src/V3AstInlines.h +++ b/src/V3AstInlines.h @@ -163,11 +163,11 @@ bool AstVar::sameNode(const AstNode* samep) const { return m_name == asamep->m_name && varType() == asamep->varType(); } -AstDecoder::AstDecoder(FileLine* fl, AstNodeExpr* indexp, AstVarScope* matchp, AstVarScope* valuep) - : ASTGEN_SUPER_Decoder(fl) { - this->indexp(indexp); +AstMatchMasked::AstMatchMasked(FileLine* fl, AstNodeExpr* lhsp, AstVarScope* matchp) + : ASTGEN_SUPER_MatchMasked(fl) { + this->lhsp(lhsp); this->matchp(new AstVarRef{fl, matchp, VAccess::READ}); - this->valuep(new AstVarRef{fl, valuep, VAccess::READ}); + dtypeSetUInt32(); } AstVarRef::AstVarRef(FileLine* fl, AstVar* varp, const VAccess& access) diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index e66c16428..91d689227 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -1328,22 +1328,6 @@ public: string emitC() override { return "VL_CVT_UNPACK_TO_Q(%P, %li)"; } bool cleanOut() const override { return true; } }; -class AstDecoder final : public AstNodeExpr { - // This is a non-source construct, created internally to represent - // some case statements. It is a '(mask & _) == bits' matching loop - // where {mask, bits} pairs are packed into a single array 'matchp', - // and the result is the corresponding value from 'valuep' for the - // first matching pair. See VL_DECODER_* runtime functions. - // @astgen op1 := indexp : AstNodeExpr - // @astgen op2 := matchp : AstVarRef - // @astgen op3 := valuep : AstVarRef -public: - inline AstDecoder(FileLine* fl, AstNodeExpr* indexp, AstVarScope* matchp, AstVarScope* valuep); - ASTGEN_MEMBERS_AstDecoder; - string emitVerilog() override { V3ERROR_NA_RETURN(""); } - string emitC() override { V3ERROR_NA_RETURN(""); } - bool cleanOut() const override { return true; } -}; class AstDist final : public AstNodeExpr { // @astgen op1 := exprp : AstNodeExpr // @astgen op2 := itemsp : List[AstDistItem] @@ -1871,6 +1855,22 @@ public: bool index() const { return m_index; } bool isExprCoverageEligible() const override { return false; } }; +class AstMatchMasked final : public AstNodeExpr { + // This is a non-source construct, created internally to represent + // some case statements. It is a '(mask & _) == bits' matching loop + // where {mask, bits} pairs are packed into a single wide 'matchp', + // and the result is the index of the first matching entry. + // See VL_DECODER_* runtime functions. + // @astgen op1 := lhsp : AstNodeExpr + // @astgen op2 := matchp : AstVarRef +public: + inline AstMatchMasked(FileLine* fl, AstNodeExpr* lhsp, AstVarScope* matchp); + ASTGEN_MEMBERS_AstMatchMasked; + string emitVerilog() override { V3ERROR_NA_RETURN(""); } + string emitC() override { return "VL_MATCHMASKED_%lq(%lw, %li, %ri)"; } + bool cleanOut() const override { return true; } + static uint32_t fold(const V3Number& lhs, AstVar* matchVarp); +}; class AstMatches final : public AstNodeExpr { // "matches" operator: "expr matches pattern" // @astgen op1 := lhsp : AstNodeExpr // Expression to match diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 3b35037a8..85b077053 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -546,6 +546,26 @@ AstConst* AstConst::parseParamLiteral(FileLine* fl, const string& literal) { string AstConstraintRef::name() const { return constrp()->name(); } +uint32_t AstMatchMasked::fold(const V3Number& lhs, AstVar* matchVarp) { + const V3Number& numTable = VN_AS(matchVarp->valuep(), Const)->num(); + V3Number numMask{matchVarp, lhs.width(), 0}; + V3Number numBits{matchVarp, lhs.width(), 0}; + V3Number numAnd{matchVarp, lhs.width(), 0}; + const int width = lhs.width(); + const int entryWidth = VL_WORDS_I(width) * VL_EDATASIZE; + uint32_t i = 0; + while (true) { + const int lsb = 2 * i * entryWidth; + const int msb = lsb + width - 1; + numMask.opSel(numTable, msb, lsb); + numBits.opSel(numTable, msb + entryWidth, lsb + entryWidth); + numAnd.opAnd(numMask, lhs); + if (numAnd.isCaseEq(numBits)) break; + ++i; + } + return i; +} + AstNetlist::AstNetlist() : ASTGEN_SUPER_Netlist(new FileLine{FileLine::builtInFilename()}) , m_typeTablep{new AstTypeTable{fileline()}} diff --git a/src/V3Case.cpp b/src/V3Case.cpp index b8b3ed776..98141623d 100644 --- a/src/V3Case.cpp +++ b/src/V3Case.cpp @@ -548,7 +548,7 @@ class CaseVisitor final : public VNVisitor { return; } - // Can optimize as AstDecoder, no other info needed + // Can optimize as AstMatchMasked, no other info needed } // Analyze case statement. Updates 'm_case*' members. Reports warnings. @@ -816,6 +816,14 @@ class CaseVisitor final : public VNVisitor { matchp->num().opSelInto(numZero, entryLsb + matchWidth / 2, condWidth); } + // Create the table initializer + AstRange* const rangep = new AstRange{flp, decoderEnries - 1, 0}; + AstNodeDType* const entryDtypep = nodep->findBitDType( + m_caseDecoderEntryWidth, m_caseDecoderEntryWidth, VSigning::UNSIGNED); + AstNodeDType* const tableDtypep = new AstUnpackArrayDType{flp, entryDtypep, rangep}; + v3Global.rootp()->typeTablep()->addTypesp(tableDtypep); + AstInitArray* const tablep = new AstInitArray{flp, tableDtypep, nullptr}; + // Build a single value table for all LHSs: one entry per clause, packing each LHS's value // at its offset. The entry width is the table packing computed by 'analyzeDecoderPattern' // Rounded up to a whole EDATA word boundary to avoid bit swizzling at runtime. @@ -824,6 +832,8 @@ class CaseVisitor final : public VNVisitor { = new AstConst{flp, AstConst::WidthedValue{}, decoderEnries * valueWidth, 0}; for (int i = 0; i < decoderEnries; ++i) { AstNode* const stmtsp = clauses[i].second; + AstConst* const entryp = new AstConst{flp, AstConst::WidthedValue{}, + static_cast(m_caseDecoderEntryWidth), 0}; for (const LhsRecord& lhsRecord : m_caseDecoderRecords) { AstNodeExpr* const lhsp = lhsRecord.lhsp; // Find the value assigned to this LHS in the clause's statements @@ -840,23 +850,24 @@ class CaseVisitor final : public VNVisitor { "Decoder LHS unassigned in case item without a pre-default"); valConstp = VN_AS(lhsRecord.preDefaultp->rhsp(), Const); } - valuep->num().opSelInto(valConstp->num(), i * valueWidth + lhsRecord.offset, - lhsp->width()); + entryp->num().opSelInto(valConstp->num(), lhsRecord.offset, lhsp->width()); } + tablep->addIndexValuep(i, entryp); } // Create the tables AstVarScope* const matchVscp = v3Global.rootp()->constPoolp()->findConst(matchp, true); - AstVarScope* const valueVscp = v3Global.rootp()->constPoolp()->findConst(valuep, true); + AstVarScope* const tableVscp = v3Global.rootp()->constPoolp()->findTable(tablep); VL_DO_DANGLING(matchp->deleteTree(), matchp); - VL_DO_DANGLING(valuep->deleteTree(), valuep); + VL_DO_DANGLING(tablep->deleteTree(), valuep); - // The decoder produces one packed entry holding every LHS's value - AstNodeExpr* const indexp = nodep->exprp()->cloneTreePure(false); - AstDecoder* const decoderp = new AstDecoder{flp, indexp, matchVscp, valueVscp}; - decoderp->dtypeSetLogicSized(m_caseDecoderEntryWidth, VSigning::UNSIGNED); + // AstMatchMasked produces the index of the matching entry + AstNodeExpr* const tableRefp = new AstVarRef{flp, tableVscp, VAccess::READ}; + AstNodeExpr* const caseExprp = nodep->exprp()->cloneTreePure(false); + AstMatchMasked* const indexp = new AstMatchMasked{flp, caseExprp, matchVscp}; + AstNodeExpr* const entryp = new AstArraySel{flp, tableRefp, indexp}; - return connectDecoderOutputs(nodep, decoderp, "__VcaseDecoderOut"); + return connectDecoderOutputs(nodep, entryp, "__VcaseDecoderOut"); } // TODO: should return AstNodeStmt after #6280 diff --git a/src/V3Clean.cpp b/src/V3Clean.cpp index 7a85dbc6b..07920f8de 100644 --- a/src/V3Clean.cpp +++ b/src/V3Clean.cpp @@ -232,9 +232,9 @@ class CleanVisitor final : public VNVisitor { ensureClean(nodep->rhsp()); setClean(nodep, true); } - void visit(AstDecoder* nodep) override { + void visit(AstMatchMasked* nodep) override { iterateChildren(nodep); - ensureClean(nodep->indexp()); + ensureClean(nodep->lhsp()); setClean(nodep, true); } void visit(AstSel* nodep) override { diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 69e8959a7..739313ff8 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -3020,10 +3020,12 @@ class ConstVisitor final : public VNVisitor { } void visit(AstClassOrPackageRef* nodep) override { iterateChildren(nodep); } - void visit(AstDecoder* nodep) override { + void visit(AstMatchMasked* nodep) override { // Do not iterate the tables, they must be constant pool entries - iterate(nodep->indexp()); - // TODO: constant fold + iterate(nodep->lhsp()); + if (AstConst* const constp = VN_CAST(nodep->lhsp(), Const)) { + replaceNum(nodep, AstMatchMasked::fold(constp->num(), nodep->matchp()->varp())); + } } void visit(AstPin* nodep) override { iterateChildren(nodep); } @@ -3288,7 +3290,8 @@ class ConstVisitor final : public VNVisitor { iterateChildren(nodep); UASSERT_OBJ(nodep->varp(), nodep, "Not linked"); bool did = false; - if (m_doV && !nodep->varp()->constPoolEntry() && nodep->varp()->valuep() && !m_attrp) { + if (m_doV && (!nodep->varp()->constPoolEntry() || m_selp) && nodep->varp()->valuep() + && !m_attrp) { // UINFOTREE(1, valuep, "", "visitvaref"); iterateAndNextNull(nodep->varp()->valuep()); // May change nodep->varp()->valuep() AstNode* const valuep = nodep->varp()->valuep(); diff --git a/src/V3Dfg.cpp b/src/V3Dfg.cpp index bb13813a5..cb18ba704 100644 --- a/src/V3Dfg.cpp +++ b/src/V3Dfg.cpp @@ -96,8 +96,8 @@ std::unique_ptr DfgGraph::clone() const { vtxp2clonep.emplace(&vtx, cp); break; } // LCOV_EXCL_STOP - case VDfgType::Decoder: { - DfgDecoder* const cp = new DfgDecoder{*clonep, vtx.fileline(), vtx.dtype()}; + case VDfgType::MatchMasked: { + DfgMatchMasked* const cp = new DfgMatchMasked{*clonep, vtx.fileline(), vtx.dtype()}; vtxp2clonep.emplace(&vtx, cp); break; } @@ -671,22 +671,21 @@ void DfgVertex::typeCheck(const DfgGraph& dfg) const { CHECK(v.srcp()->dtype() == v.dtype().elemDtype(), "Input should be the element type"); return; } - case VDfgType::Decoder: { - const DfgDecoder& v = *as(); - CHECK(v.isPacked(), "Should be Packed type"); - CHECK(v.indexp()->isPacked(), "Index should be Packed type"); - CHECK(v.matchp()->isPacked(), "Match should be Packed type"); - CHECK(v.valuep()->isPacked(), "Value should be Packed type"); - CHECK(v.matchp()->is(), "Match should be a variable"); - CHECK(v.matchp()->is(), "Value should be a variable"); - return; - } case VDfgType::Sel: { const DfgSel& v = *as(); CHECK(v.isPacked(), "Should be Packed type"); CHECK(v.dtype() == DfgDataType::select(v.srcp()->dtype(), v.lsb(), v.size()), "sel"); return; } + case VDfgType::MatchMasked: { + const DfgMatchMasked& v = *as(); + CHECK(v.isPacked(), "Should be Packed type"); + CHECK(v.size() == 32U, "Should yield a 32-bit result"); + CHECK(v.lhsp()->isPacked(), "Lhs should be packed"); + CHECK(v.matchp()->isPacked(), "Match should be Packed type"); + CHECK(v.matchp()->is(), "Match should be a variable"); + return; + } case VDfgType::Mux: { const DfgMux& v = *as(); CHECK(v.isPacked(), "Should be Packed type"); diff --git a/src/V3Dfg.h b/src/V3Dfg.h index 95320daad..b93c1c716 100644 --- a/src/V3Dfg.h +++ b/src/V3Dfg.h @@ -817,8 +817,11 @@ bool DfgVertex::isCheaperThanLoad() const { if (is()) return true; // Variables if (is()) return true; - // Array sels are just address computation - if (is()) return true; + // Array sels are just address computation, but the address itself can be expensive + if (const DfgArraySel* aselp = cast()) { + if (aselp->bitp()->is()) return false; + return true; + } // Small select from variable if (const DfgSel* const selp = cast()) { if (!selp->fromp()->is()) return false; diff --git a/src/V3DfgCse.cpp b/src/V3DfgCse.cpp index 8f666b160..c1b3862e4 100644 --- a/src/V3DfgCse.cpp +++ b/src/V3DfgCse.cpp @@ -72,7 +72,7 @@ class V3DfgCse final { } // Vertices with no internal information - case VDfgType::Decoder: + case VDfgType::MatchMasked: case VDfgType::Mux: case VDfgType::UnitArray: return V3Hash{}; @@ -198,7 +198,7 @@ class V3DfgCse final { } // Vertices with no internal information - case VDfgType::Decoder: + case VDfgType::MatchMasked: case VDfgType::Mux: case VDfgType::UnitArray: return true; diff --git a/src/V3DfgDfgToAst.cpp b/src/V3DfgDfgToAst.cpp index f334b36e0..b2af96461 100644 --- a/src/V3DfgDfgToAst.cpp +++ b/src/V3DfgDfgToAst.cpp @@ -234,13 +234,11 @@ class DfgToAstVisitor final : DfgVisitor { AstVar* const varp = sinkp->as()->vscp()->varp(); m_resultp = new AstCReset{vtxp->fileline(), varp, false}; } - void visit(DfgDecoder* vtxp) override { + void visit(DfgMatchMasked* vtxp) override { FileLine* const flp = vtxp->fileline(); - AstNodeExpr* const indexp = convertDfgVertexToAstNodeExpr(vtxp->indexp()); + AstNodeExpr* const lhsp = convertDfgVertexToAstNodeExpr(vtxp->lhsp()); AstVarScope* const matchp = vtxp->matchp()->as()->vscp(); - AstVarScope* const valuep = vtxp->valuep()->as()->vscp(); - m_resultp = new AstDecoder{flp, indexp, matchp, valuep}; - m_resultp->dtypeSetLogicSized(vtxp->width(), VSigning::UNSIGNED); + m_resultp = new AstMatchMasked{flp, lhsp, matchp}; } void visit(DfgRep* vtxp) override { diff --git a/src/V3DfgPeephole.cpp b/src/V3DfgPeephole.cpp index 3f08f490f..de92790a8 100644 --- a/src/V3DfgPeephole.cpp +++ b/src/V3DfgPeephole.cpp @@ -2751,6 +2751,16 @@ class V3DfgPeephole final : public DfgVisitor { } } + void visit(DfgMatchMasked* const vtxp) override { + if (DfgConst* const constp = vtxp->lhsp()->cast()) { + APPLYING(FOLD_MATCHMASKED) { + AstVar* const matchVarp = vtxp->matchp()->as()->vscp()->varp(); + replace(makeI32(vtxp->fileline(), AstMatchMasked::fold(constp->num(), matchVarp))); + return; + } + } + } + //========================================================================= // DfgVertexTernary //========================================================================= diff --git a/src/V3DfgPeepholePatterns.h b/src/V3DfgPeepholePatterns.h index 52d5717fe..1c9de14e2 100644 --- a/src/V3DfgPeepholePatterns.h +++ b/src/V3DfgPeepholePatterns.h @@ -31,6 +31,7 @@ _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, FOLD_ASSOC_BINARY_LHS_OF_RHS) \ _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, FOLD_ASSOC_BINARY_RHS_OF_LHS) \ _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, FOLD_BINARY) \ + _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, FOLD_MATCHMASKED) \ _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, FOLD_MUX_FROM_ONES) \ _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, FOLD_MUX_FROM_ZERO) \ _FOR_EACH_DFG_PEEPHOLE_OPTIMIZATION_APPLY(macro, FOLD_REP) \ diff --git a/src/V3DfgSynthesize.cpp b/src/V3DfgSynthesize.cpp index 677335af8..c0f5bdc99 100644 --- a/src/V3DfgSynthesize.cpp +++ b/src/V3DfgSynthesize.cpp @@ -402,7 +402,7 @@ class AstToDfgConverter final : public VNVisitor { DfgVertex* const vtxp = make(nodep->fileline(), *dtypep); nodep->user2p(vtxp); } - void visit(AstDecoder* nodep) override { + void visit(AstMatchMasked* nodep) override { UASSERT_OBJ(m_converting, nodep, "AstToDfg visit called without m_converting"); UASSERT_OBJ(!nodep->user2p(), nodep, "Already has Dfg vertex"); if (unhandled(nodep)) return; @@ -414,18 +414,15 @@ class AstToDfgConverter final : public VNVisitor { return; } - iterate(nodep->indexp()); + iterate(nodep->lhsp()); if (m_foundUnhandled) return; iterate(nodep->matchp()); if (m_foundUnhandled) return; - iterate(nodep->valuep()); - if (m_foundUnhandled) return; FileLine* const flp = nodep->fileline(); - DfgDecoder* const vtxp = make(flp, *dtypep); - vtxp->indexp(nodep->indexp()->user2u().to()); + DfgMatchMasked* const vtxp = make(flp, *dtypep); + vtxp->lhsp(nodep->lhsp()->user2u().to()); vtxp->matchp(nodep->matchp()->user2u().to()); - vtxp->valuep(nodep->valuep()->user2u().to()); nodep->user2p(vtxp); } void visit(AstReplicate* nodep) override { diff --git a/src/V3DfgVertices.h b/src/V3DfgVertices.h index 12d3b00cb..d932de1a3 100644 --- a/src/V3DfgVertices.h +++ b/src/V3DfgVertices.h @@ -329,6 +329,21 @@ public: ASTGEN_MEMBERS_DfgVertexBinary; }; +class DfgMatchMasked final : public DfgVertexBinary { + // Dfg equivalent of AstMatchMasked +public: + DfgMatchMasked(DfgGraph& dfg, FileLine* flp, const DfgDataType& dtype) + : DfgVertexBinary{dfg, dfgType(), flp, dtype} {} + ASTGEN_MEMBERS_DfgMatchMasked; + + DfgVertex* lhsp() const { return inputp(0); } + void lhsp(DfgVertex* vtxp) { inputp(0, vtxp); } + DfgVertex* matchp() const { return inputp(1); } + void matchp(DfgVertex* vtxp) { inputp(1, vtxp); } + + std::string srcName(size_t idx) const override { return idx ? "matchp" : "lhsp"; } +}; + class DfgMux final : public DfgVertexBinary { // AstSel is binary, but 'lsbp' is very often constant. As AstSel is fairly // common, we special case as a DfgSel for the constant 'lsbp', and as @@ -362,24 +377,6 @@ public: ASTGEN_MEMBERS_DfgVertexTernary; }; -class DfgDecoder final : public DfgVertexTernary { -public: - DfgDecoder(DfgGraph& dfg, FileLine* flp, const DfgDataType& dtype) - : DfgVertexTernary{dfg, dfgType(), flp, dtype} {} - ASTGEN_MEMBERS_DfgDecoder; - - DfgVertex* indexp() const { return inputp(0); } - void indexp(DfgVertex* vtxp) { inputp(0, vtxp); } - DfgVertex* matchp() const { return inputp(1); } - void matchp(DfgVertex* vtxp) { inputp(1, vtxp); } - DfgVertex* valuep() const { return inputp(2); } - void valuep(DfgVertex* vtxp) { inputp(2, vtxp); } - - std::string srcName(size_t idx) const override { - return idx == 0 ? "indexp" : idx == 1 ? "matchp" : "valuep"; - } -}; - //------------------------------------------------------------------------------ // Variadic vertices - variable number of inputs diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index b705cecc7..5e527870c 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -1590,27 +1590,8 @@ public: puts(")"); } } - void visit(AstDecoder* nodep) override { - putns(nodep, "VL_DECODER_"); - emitIQW(nodep); - emitIQW(nodep->indexp()); - puts("("); - if (nodep->isWide()) { - puts(std::to_string(nodep->widthWords())); - puts(", "); - iterateConst(m_wideTempRefp); - puts(", "); - } - if (nodep->indexp()->isWide()) { - puts(std::to_string(nodep->indexp()->widthWords())); - puts(", "); - } - iterateConst(nodep->indexp()); - puts(", "); - iterateConst(nodep->matchp()); - puts(", "); - iterateConst(nodep->valuep()); - puts(")"); + void visit(AstMatchMasked* nodep) override { + emitOpName(nodep, nodep->emitC(), nodep->lhsp(), nodep->matchp(), nullptr); } void visit(AstMemberSel* nodep) override { iterateAndNextConstNull(nodep->fromp()); diff --git a/src/V3Gate.cpp b/src/V3Gate.cpp index b96c892f2..e41b61daf 100644 --- a/src/V3Gate.cpp +++ b/src/V3Gate.cpp @@ -471,6 +471,11 @@ class GateOkVisitor final : public VNVisitorConst { // assign to get randomization etc clearSimple("CReset"); } + void visit(AstMatchMasked* nodep) override { + if (!m_isSimple) return; + // This node can be expensive + clearSimple("MatchMasked"); + } //-------------------- void visit(AstNode* nodep) override { if (!m_isSimple) return; // Fastpath diff --git a/src/V3Premit.cpp b/src/V3Premit.cpp index 81c2dda3e..ca09e98ba 100644 --- a/src/V3Premit.cpp +++ b/src/V3Premit.cpp @@ -347,9 +347,13 @@ class PremitVisitor final : public VNVisitor { } checkNode(nodep); } - void visit(AstDecoder* nodep) override { + void visit(AstMatchMasked* nodep) override { iterateChildren(nodep); - checkNode(nodep); + if (!nodep->user1SetOnce()) { + // Don't want this replicated by V3Expand + AstVar* const varp = createTemp(nodep); + varp->noSubst(true); // Do not re-inline in V3Subst + } } void visit(AstCond* nodep) override { // Convert AstCond to AstIf in order to avoid evaluating diff --git a/test_regress/t/t_case_decoder.py b/test_regress/t/t_case_decoder.py index 2949ecc37..4265c138b 100755 --- a/test_regress/t/t_case_decoder.py +++ b/test_regress/t/t_case_decoder.py @@ -15,6 +15,6 @@ test.compile(verilator_flags2=['--binary', '--stats']) test.execute() -test.file_grep(test.stats, r'Optimizations, Cases decoder\s+(\d+)', 16) +test.file_grep(test.stats, r'Optimizations, Cases decoder\s+(\d+)', 17) test.passes() diff --git a/test_regress/t/t_case_decoder.v b/test_regress/t/t_case_decoder.v index 76288d76c..6d5c779c2 100644 --- a/test_regress/t/t_case_decoder.v +++ b/test_regress/t/t_case_decoder.v @@ -444,6 +444,22 @@ module t; assign accept_j_ref = cyc[4:0] == 5'd1 ? 6'd1 : cyc[4:0] == 5'd2 ? 6'd2 : cyc[4:0] == 5'd3 ? 6'd3 : ~6'd0; + // Accept K: Will constant fold after converting to decoder + wire [23:0] accept_k_in; + logic [5:0] accept_k_out, accept_k_ref; + always_ff @(posedge clk) begin + accept_k_out <= '1; + casez (accept_k_in) + 24'b????????_????????_???????1 : accept_k_out <= 6'd0; + 24'b????????_????????_??????1? : accept_k_out <= 6'd1; + 24'b????????_????????_?????1?? : accept_k_out <= 6'd2; + 24'b????????_????????_????1??? : accept_k_out <= 6'd3; + endcase + end + always_ff @(posedge clk) + accept_k_ref <= 6'd2; + assign accept_k_in = 24'b100; + // The cases below are intentionally NOT converted to a decoder. // Reject A: too few conditions to be worth the indexed load. @@ -495,6 +511,7 @@ module t; `checkh(accept_i_out_0, accept_i_ref_0); `checkh(accept_i_out_1, accept_i_ref_1); `checkh(accept_j_out, accept_j_ref); + `checkh(accept_k_out, accept_k_ref); `checkh(reject_a_out, reject_a_ref); `checkh(reject_b_out, reject_b_ref); diff --git a/test_regress/t/t_dfg_peephole.v b/test_regress/t/t_dfg_peephole.v index 98d9146be..3e393a297 100644 --- a/test_regress/t/t_dfg_peephole.v +++ b/test_regress/t/t_dfg_peephole.v @@ -136,12 +136,12 @@ module t ( `signal(FOLD_SEL, const_a[3:1]); - int fold_arraysel_table_one; - ffs ffs_a(convoluted_zero[0] ? 8'hff: 8'd2, fold_arraysel_table_one); - int fold_arraysel_table_two; - ffs ffs_b(convoluted_zero[1] ? 8'hff: 8'd7, fold_arraysel_table_two); - `signal(FOLD_ARRAYSEL_TABLE_ONE, fold_arraysel_table_one); - `signal(FOLD_ARRAYSEL_TABLE_TWO, fold_arraysel_table_two); + int fold_arraysel_table; + ffs ffs_a(convoluted_zero[0] ? 20'hff: 20'd2, fold_arraysel_table); + int fold_matchmasked; + ffs ffs_b(convoluted_zero[1] ? 20'hff: 20'd7, fold_matchmasked); + `signal(FOLD_ARRAYSEL_TABLE, fold_arraysel_table); + `signal(FOLD_MATCHMASKED, fold_matchmasked); `signal(SWAP_CONST_IN_COMMUTATIVE_BINARY, rand_a + const_a); `signal(SWAP_NOT_IN_COMMUTATIVE_BINARY, rand_a + ~rand_a); @@ -439,23 +439,33 @@ module t ( endmodule module ffs( - input logic [7:0] i, + input logic [19:0] i, output int o ); // V3Table will convert this always_comb begin - // verilator lint_off CASEOVERLAP casez (i) - 8'b1???????: o = 7; - 8'b?1??????: o = 6; - 8'b??1?????: o = 5; - 8'b???1????: o = 4; - 8'b????1???: o = 3; - 8'b?????1??: o = 2; - 8'b??????1?: o = 1; - 8'b???????1: o = 0; - 8'b00000000: o = -1; + 20'b1???????????????????: o = 19; + 20'b?1??????????????????: o = 18; + 20'b??1?????????????????: o = 17; + 20'b???1????????????????: o = 16; + 20'b????1???????????????: o = 15; + 20'b?????1??????????????: o = 14; + 20'b??????1?????????????: o = 13; + 20'b???????1????????????: o = 12; + 20'b????????1???????????: o = 11; + 20'b?????????1??????????: o = 10; + 20'b??????????1?????????: o = 9; + 20'b???????????1????????: o = 8; + 20'b????????????1???????: o = 7; + 20'b?????????????1??????: o = 6; + 20'b??????????????1?????: o = 5; + 20'b???????????????1????: o = 4; + 20'b????????????????1???: o = 3; + 20'b?????????????????1??: o = 2; + 20'b??????????????????1?: o = 1; + 20'b???????????????????1: o = 0; + default: o = 32'hffffffff; endcase - // verilator lint_on CASEOVERLAP end endmodule