DFG: Support AstSel and AstConcat on LHS of assignments

Added DfgVertexVariadic to represent DFG vetices with a varying number
of source operands. Converted DfgVar to be a variadic vertex, with each
driver corresponding to a fixed range of bits in the packed variable.
This allows us to handle AstSel on the LHS of assignments. Also added
support for AstConcat on the LHS by selecting into the RHS as
appropriate.

This improves OpenTitan ST speed by ~13%

src/V3Dfg.cpp

@@ -251,7 +251,7 @@ static void dumpDotVertexAndSourceEdges(std::ostream& os, const DfgVertex& vtx)
     vtx.forEachSourceEdge([&](const DfgEdge& edge, size_t idx) {  //
         if (edge.sourcep()) {
             string headLabel;
-            if (vtx.arity() > 1) headLabel = std::toupper(vtx.srcName(idx)[0]);
+            if (vtx.arity() > 1) headLabel = vtx.srcName(idx);
             dumpDotEdge(os, edge, headLabel);
         }
     });
@@ -502,11 +502,15 @@ void DfgVertex::replaceWith(DfgVertex* newSorucep) {
 void DfgVar::accept(DfgVisitor& visitor) { visitor.visit(this); }
 
 bool DfgVar::selfEquals(const DfgVertex& that) const {
-    if (const DfgVar* otherp = that.cast<DfgVar>()) return varp() == otherp->varp();
+    if (const DfgVar* otherp = that.cast<DfgVar>()) {
+        UASSERT_OBJ(varp() != otherp->varp() || this == otherp, this,
+                    "There should only be one DfgVar for a given AstVar");
+        return this == otherp;
+    }
     return false;
 }
 
-V3Hash DfgVar::selfHash() const { return V3Hasher::uncachedHash(m_varp); }
+V3Hash DfgVar::selfHash() const { return V3Hasher::uncachedHash(varp()); }
 
 // DfgConst ----------
 void DfgConst::accept(DfgVisitor& visitor) { visitor.visit(this); }

src/V3Dfg.h

@@ -39,6 +39,7 @@
 #include "V3Hasher.h"
 #include "V3List.h"
 
+#include <array>
 #include <functional>
 #include <type_traits>
 #include <unordered_map>
@@ -157,18 +158,18 @@ public:
 
 class DfgEdge final {
     friend class DfgVertex;
-    template <size_t Arity>
-    friend class DfgVertexWithArity;
 
     DfgEdge* m_nextp = nullptr;  // Next edge in sink list
     DfgEdge* m_prevp = nullptr;  // Previous edge in sink list
     DfgVertex* m_sourcep = nullptr;  // The source vertex driving this edge
-    DfgVertex* const m_sinkp;  // The sink vertex. The sink owns the edge, so immutable
-
-    explicit DfgEdge(DfgVertex* sinkp)  // The sink vertices own the edges, hence private
-        : m_sinkp{sinkp} {}
+    // Note that the sink vertex owns the edge, so it is immutable, but because we want to be able
+    // to allocate these as arrays, we use a default constructor + 'init' method to set m_sinkp.
+    DfgVertex* const m_sinkp = nullptr;  // The sink vertex
 
 public:
+    DfgEdge() {}
+    void init(DfgVertex* sinkp) { const_cast<DfgVertex*&>(m_sinkp) = sinkp; }
+
     // The source (driver) of this edge
     DfgVertex* sourcep() const { return m_sourcep; }
     // The sink (consumer) of this edge
@@ -290,10 +291,10 @@ public:
     }
 
     // Source edges of this vertex
-    virtual std::pair<DfgEdge*, size_t> sourceEdges() { return {nullptr, 0}; }
+    virtual std::pair<DfgEdge*, size_t> sourceEdges() = 0;
 
     // Source edges of this vertex
-    virtual std::pair<const DfgEdge*, size_t> sourceEdges() const { return {nullptr, 0}; }
+    virtual std::pair<const DfgEdge*, size_t> sourceEdges() const = 0;
 
     // Arity (number of sources) of this vertex
     size_t arity() const { return sourceEdges().second; }
@@ -429,45 +430,42 @@ template <size_t Arity>
 class DfgVertexWithArity VL_NOT_FINAL : public DfgVertex {
     static_assert(1 <= Arity && Arity <= 4, "Arity must be between 1 and 4 inclusive");
 
-    // Uninitialized storage for source edges
-    typename std::aligned_storage<sizeof(DfgEdge[Arity]), alignof(DfgEdge[Arity])>::type
-        m_sourceEdges;
-
-    constexpr DfgEdge& sourceEdge(size_t index) {
-        return reinterpret_cast<DfgEdge*>(&m_sourceEdges)[index];
-    }
-    constexpr const DfgEdge& sourceEdge(size_t index) const {
-        return reinterpret_cast<const DfgEdge*>(&m_sourceEdges)[index];
-    }
+    std::array<DfgEdge, Arity> m_srcs;  // Source edges
 
 protected:
     DfgVertexWithArity<Arity>(DfgGraph& dfg, FileLine* flp, AstNodeDType* dtypep, DfgType type)
         : DfgVertex{dfg, flp, dtypep, type} {
         // Initialize source edges
-        for (size_t i = 0; i < Arity; ++i) new (&sourceEdge(i)) DfgEdge{this};
+        for (size_t i = 0; i < Arity; ++i) m_srcs[i].init(this);
     }
 
-    virtual ~DfgVertexWithArity<Arity>() = default;
+    ~DfgVertexWithArity<Arity>() override = default;
 
 public:
     std::pair<DfgEdge*, size_t> sourceEdges() override {  //
-        return {&sourceEdge(0), Arity};
+        return {m_srcs.data(), Arity};
     }
     std::pair<const DfgEdge*, size_t> sourceEdges() const override {
-        return {&sourceEdge(0), Arity};
+        return {m_srcs.data(), Arity};
+    }
+
+    template <size_t Index>
+    DfgEdge* sourceEdge() {
+        static_assert(Index < Arity, "Source index out of range");
+        return &m_srcs[Index];
     }
 
     template <size_t Index>
     DfgVertex* source() const {
         static_assert(Index < Arity, "Source index out of range");
-        return sourceEdge(Index).m_sourcep;
+        return m_srcs[Index].sourcep();
     }
 
     template <size_t Index>
     void relinkSource(DfgVertex* newSourcep) {
         static_assert(Index < Arity, "Source index out of range");
-        UASSERT_OBJ(sourceEdge(Index).m_sinkp == this, this, "Inconsistent");
-        sourceEdge(Index).relinkSource(newSourcep);
+        UASSERT_OBJ(m_srcs[Index].sinkp() == this, this, "Inconsistent");
+        m_srcs[Index].relinkSource(newSourcep);
     }
 
     // Named source getter/setter for unary vertices
@@ -506,41 +504,89 @@ public:
     }
 };
 
+class DfgVertexVariadic VL_NOT_FINAL : public DfgVertex {
+    DfgEdge* m_srcsp;  // The source edges
+    uint32_t m_srcCnt = 0;  // Number of sources used
+    uint32_t m_srcCap;  // Number of sources allocated
+
+    // Allocate a new source edge array
+    DfgEdge* allocSources(size_t n) {
+        DfgEdge* const srcsp = new DfgEdge[n];
+        for (size_t i = 0; i < n; ++i) srcsp[i].init(this);
+        return srcsp;
+    }
+
+    // Double the capacity of m_srcsp
+    void growSources() {
+        m_srcCap *= 2;
+        DfgEdge* const newsp = allocSources(m_srcCap);
+        for (size_t i = 0; i < m_srcCnt; ++i) {
+            DfgEdge* const oldp = m_srcsp + i;
+            // Skip over unlinked source edge
+            if (!oldp->sourcep()) continue;
+            // New edge driven from the same vertex as the old edge
+            newsp[i].relinkSource(oldp->sourcep());
+            // Unlink the old edge, it will be deleted
+            oldp->unlinkSource();
+        }
+        // Delete old source edges
+        delete[] m_srcsp;
+        // Keep hold of new source edges
+        m_srcsp = newsp;
+    }
+
+protected:
+    DfgVertexVariadic(DfgGraph& dfg, FileLine* flp, AstNodeDType* dtypep, DfgType type,
+                      uint32_t initialCapacity = 1)
+        : DfgVertex{dfg, flp, dtypep, type}
+        , m_srcsp{allocSources(initialCapacity)}
+        , m_srcCap{initialCapacity} {}
+
+    ~DfgVertexVariadic() override { delete[] m_srcsp; };
+
+    DfgEdge* addSource() {
+        if (m_srcCnt == m_srcCap) growSources();
+        return m_srcsp + m_srcCnt++;
+    }
+
+    void resetSources() {
+        // #ifdef VL_DEBUG TODO: DEBUG ONLY
+        for (uint32_t i = 0; i < m_srcCnt; ++i) {
+            UASSERT_OBJ(!m_srcsp[i].sourcep(), m_srcsp[i].sourcep(), "Connected source");
+        }
+        // #endif
+        m_srcCnt = 0;
+    }
+
+public:
+    DfgEdge* sourceEdge(size_t idx) const { return &m_srcsp[idx]; }
+    DfgVertex* source(size_t idx) const { return m_srcsp[idx].sourcep(); }
+
+    std::pair<DfgEdge*, size_t> sourceEdges() override { return {m_srcsp, m_srcCnt}; }
+    std::pair<const DfgEdge*, size_t> sourceEdges() const override { return {m_srcsp, m_srcCnt}; }
+};
+
 //------------------------------------------------------------------------------
 // Vertex classes
 //------------------------------------------------------------------------------
 
-class DfgVar final : public DfgVertexWithArity<1> {
-    friend class DfgVertex;
-    friend class DfgVisitor;
-
+class DfgVertexLValue VL_NOT_FINAL : public DfgVertexVariadic {
     AstVar* const m_varp;  // The AstVar associated with this vertex (not owned by this vertex)
-    FileLine* m_assignmentFlp;  // The FileLine of the original assignment driving this var
     bool m_hasModRefs = false;  // This AstVar is referenced outside the DFG, but in the module
     bool m_hasExtRefs = false;  // This AstVar is referenced from outside the module
 
-    void accept(DfgVisitor& visitor) override;
-    bool selfEquals(const DfgVertex& that) const override;
-    V3Hash selfHash() const override;
-    static constexpr DfgType dfgType() { return DfgType::atVar; };
-
 public:
-    DfgVar(DfgGraph& dfg, AstVar* varp)
-        : DfgVertexWithArity<1>{dfg, varp->fileline(), dtypeFor(varp), dfgType()}
+    DfgVertexLValue(DfgGraph& dfg, DfgType type, AstVar* varp, uint32_t initialCapacity)
+        : DfgVertexVariadic{dfg, varp->fileline(), dtypeFor(varp), type, initialCapacity}
         , m_varp{varp} {}
 
     AstVar* varp() const { return m_varp; }
-    FileLine* assignmentFileline() const { return m_assignmentFlp; }
-    void assignmentFileline(FileLine* flp) { m_assignmentFlp = flp; }
     bool hasModRefs() const { return m_hasModRefs; }
     void setHasModRefs() { m_hasModRefs = true; }
     bool hasExtRefs() const { return m_hasExtRefs; }
     void setHasExtRefs() { m_hasExtRefs = true; }
     bool hasRefs() const { return m_hasModRefs || m_hasExtRefs; }
 
-    DfgVertex* driverp() const { return srcp(); }
-    void driverp(DfgVertex* vtxp) { srcp(vtxp); }
-
     // Variable cannot be removed, even if redundant in the DfgGraph (might be used externally)
     bool keep() const {
         // Keep if referenced outside this module
@@ -552,8 +598,44 @@ public:
         // Otherwise it can be removed
         return false;
     }
+};
 
-    const string srcName(size_t) const override { return "driverp"; }
+class DfgVar final : public DfgVertexLValue {
+    friend class DfgVertex;
+    friend class DfgVisitor;
+
+    using DriverData = std::pair<FileLine*, uint32_t>;
+
+    std::vector<DriverData> m_driverData;  // Additional data associate with each driver
+
+    void accept(DfgVisitor& visitor) override;
+    bool selfEquals(const DfgVertex& that) const override;
+    V3Hash selfHash() const override;
+    static constexpr DfgType dfgType() { return DfgType::atVar; };
+
+public:
+    DfgVar(DfgGraph& dfg, AstVar* varp)
+        : DfgVertexLValue{dfg, dfgType(), varp, 1u} {}
+
+    bool isDrivenByDfg() const { return arity() > 0; }
+    bool isDrivenFullyByDfg() const { return arity() == 1 && source(0)->dtypep() == dtypep(); }
+
+    void addDriver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp) {
+        m_driverData.emplace_back(flp, lsb);
+        DfgVertexVariadic::addSource()->relinkSource(vtxp);
+    }
+
+    void resetSources() {
+        m_driverData.clear();
+        DfgVertexVariadic::resetSources();
+    }
+
+    FileLine* driverFileLine(size_t idx) const { return m_driverData[idx].first; }
+    uint32_t driverLsb(size_t idx) const { return m_driverData[idx].second; }
+
+    const string srcName(size_t idx) const override {
+        return isDrivenFullyByDfg() ? "" : cvtToStr(driverLsb(idx));
+    }
 };
 
 class DfgConst final : public DfgVertex {
@@ -572,7 +654,7 @@ public:
         : DfgVertex{dfg, constp->fileline(), dtypeFor(constp), dfgType()}
         , m_constp{constp} {}
 
-    ~DfgConst() { VL_DO_DANGLING(m_constp->deleteTree(), m_constp); }
+    ~DfgConst() override { VL_DO_DANGLING(m_constp->deleteTree(), m_constp); }
 
     AstConst* constp() const { return m_constp; }
     V3Number& num() const { return m_constp->num(); }
@@ -583,6 +665,8 @@ public:
     bool isZero() const { return num().isEqZero(); }
     bool isOnes() const { return num().isEqAllOnes(width()); }
 
+    std::pair<DfgEdge*, size_t> sourceEdges() override { return {nullptr, 0}; }
+    std::pair<const DfgEdge*, size_t> sourceEdges() const override { return {nullptr, 0}; }
     const string srcName(size_t) const override {  // LCOV_EXCL_START
         VL_UNREACHABLE;
         return "";

src/V3DfgAstToDfg.cpp

@@ -35,6 +35,8 @@
 #include "V3Error.h"
 #include "V3Global.h"
 
+#include <tuple>
+
 VL_DEFINE_DEBUG_FUNCTIONS;
 
 namespace {
@@ -86,6 +88,7 @@ class AstToDfgVisitor final : public VNVisitor {
     bool m_foundUnhandled = false;  // Found node not implemented as DFG or not implemented 'visit'
     std::vector<DfgVertex*> m_uncommittedVertices;  // Vertices that we might decide to revert
     bool m_converting = false;  // We are trying to convert some logic at the moment
+    std::vector<DfgVar*> m_varps;  // All the DfgVar vertices we created.
 
     // METHODS
     void markReferenced(AstNode* nodep) {
@@ -110,7 +113,9 @@ class AstToDfgVisitor final : public VNVisitor {
             // multiple AstVarRef instances, so we will never revert a DfgVar once created. This
             // means we can end up with DfgVar vertices in the graph which have no connections at
             // all (which is fine for later processing).
-            varp->user1p(new DfgVar{*m_dfgp, varp});
+            DfgVar* const vtxp = new DfgVar{*m_dfgp, varp};
+            m_varps.push_back(vtxp);
+            varp->user1p(vtxp);
         }
         return varp->user1u().to<DfgVar*>();
     }
@@ -139,6 +144,95 @@ class AstToDfgVisitor final : public VNVisitor {
         return m_foundUnhandled;
     }
 
+    // Build DfgEdge representing the LValue assignment. Returns false if unsuccessful.
+    bool convertAssignment(FileLine* flp, AstNode* nodep, DfgVertex* vtxp) {
+        if (AstVarRef* const vrefp = VN_CAST(nodep, VarRef)) {
+            m_foundUnhandled = false;
+            visit(vrefp);
+            if (m_foundUnhandled) return false;
+            getVertex(vrefp)->as<DfgVar>()->addDriver(flp, 0, vtxp);
+            return true;
+        }
+        if (AstSel* const selp = VN_CAST(nodep, Sel)) {
+            AstVarRef* const vrefp = VN_CAST(selp->fromp(), VarRef);
+            AstConst* const lsbp = VN_CAST(selp->lsbp(), Const);
+            if (!vrefp || !lsbp || !VN_IS(selp->widthp(), Const)) {
+                ++m_ctx.m_nonRepLhs;
+                return false;
+            }
+            m_foundUnhandled = false;
+            visit(vrefp);
+            if (m_foundUnhandled) return false;
+            getVertex(vrefp)->as<DfgVar>()->addDriver(flp, lsbp->toUInt(), vtxp);
+            return true;
+        }
+        if (AstConcat* const concatp = VN_CAST(nodep, Concat)) {
+            AstNode* const lhsp = concatp->lhsp();
+            AstNode* const rhsp = concatp->rhsp();
+            const uint32_t lWidth = lhsp->width();
+            const uint32_t rWidth = rhsp->width();
+
+            {
+                FileLine* const lFlp = lhsp->fileline();
+                DfgSel* const lVtxp = new DfgSel{*m_dfgp, lFlp, DfgVertex::dtypeFor(lhsp)};
+                lVtxp->fromp(vtxp);
+                lVtxp->lsbp(new DfgConst{*m_dfgp, new AstConst{lFlp, rWidth}});
+                lVtxp->widthp(new DfgConst{*m_dfgp, new AstConst{lFlp, lWidth}});
+                if (!convertAssignment(flp, lhsp, lVtxp)) return false;
+            }
+
+            {
+                FileLine* const rFlp = rhsp->fileline();
+                DfgSel* const rVtxp = new DfgSel{*m_dfgp, rFlp, DfgVertex::dtypeFor(rhsp)};
+                rVtxp->fromp(vtxp);
+                rVtxp->lsbp(new DfgConst{*m_dfgp, new AstConst{rFlp, 0u}});
+                rVtxp->widthp(new DfgConst{*m_dfgp, new AstConst{rFlp, rWidth}});
+                return convertAssignment(flp, rhsp, rVtxp);
+            }
+        }
+        ++m_ctx.m_nonRepLhs;
+        return false;
+    }
+
+    bool convertEquation(AstNode* nodep, AstNode* lhsp, AstNode* rhsp) {
+        UASSERT_OBJ(m_uncommittedVertices.empty(), nodep, "Should not nest");
+
+        // Cannot handle mismatched widths. Mismatched assignments should have been fixed up in
+        // earlier passes anyway, so this should never be hit, but being paranoid just in case.
+        if (lhsp->width() != rhsp->width()) {  // LCOV_EXCL_START
+            markReferenced(nodep);
+            ++m_ctx.m_nonRepWidth;
+            return false;
+        }  // LCOV_EXCL_STOP
+
+        VL_RESTORER(m_converting);
+        m_converting = true;
+
+        m_foundUnhandled = false;
+        iterate(rhsp);
+        if (m_foundUnhandled) {
+            revertUncommittedVertices();
+            markReferenced(nodep);
+            return false;
+        }
+
+        if (!convertAssignment(nodep->fileline(), lhsp, getVertex(rhsp))) {
+            revertUncommittedVertices();
+            markReferenced(nodep);
+            return false;
+        }
+
+        // Connect the rhs vertex to the driven edge
+        commitVertices();
+
+        // Remove node from Ast. Now represented by the Dfg.
+        VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
+
+        //
+        ++m_ctx.m_representable;
+        return true;
+    }
+
     // VISITORS
     void visit(AstNode* nodep) override {
         // Conservatively treat this node as unhandled
@@ -159,8 +253,6 @@ class AstToDfgVisitor final : public VNVisitor {
     }
 
     void visit(AstAssignW* nodep) override {
-        VL_RESTORER(m_converting);
-        m_converting = true;
         ++m_ctx.m_inputEquations;
 
         // Cannot handle assignment with timing control yet
@@ -170,50 +262,7 @@ class AstToDfgVisitor final : public VNVisitor {
             return;
         }
 
-        // Cannot handle mismatched widths. Mismatched assignments should have been fixed up in
-        // earlier passes anyway, so this should never be hit, but being paranoid just in case.
-        if (nodep->lhsp()->width() != nodep->rhsp()->width()) {  // LCOV_EXCL_START
-            markReferenced(nodep);
-            ++m_ctx.m_nonRepWidth;
-            return;
-        }  // LCOV_EXCL_START
-
-        // Simple assignment with whole variable on left-hand side
-        if (AstVarRef* const vrefp = VN_CAST(nodep->lhsp(), VarRef)) {
-            UASSERT_OBJ(m_uncommittedVertices.empty(), nodep, "Should not nest");
-
-            // Build DFG vertices representing the two sides
-            {
-                m_foundUnhandled = false;
-                iterate(vrefp);
-                iterate(nodep->rhsp());
-                // If this assignment contains an AstNode not representable by a DfgVertex,
-                // then revert the graph.
-                if (m_foundUnhandled) {
-                    revertUncommittedVertices();
-                    markReferenced(nodep);
-                    return;
-                }
-            }
-
-            // Connect the vertices representing the 2 sides
-            DfgVar* const lVtxp = getVertex(vrefp)->as<DfgVar>();
-            DfgVertex* const rVtxp = getVertex(nodep->rhsp());
-            lVtxp->driverp(rVtxp);
-            lVtxp->assignmentFileline(nodep->fileline());
-            commitVertices();
-
-            // Remove assignment from Ast. Now represented by the Dfg.
-            VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
-
-            //
-            ++m_ctx.m_representable;
-            return;
-        }
-
-        // TODO: handle complex left-hand sides
-        markReferenced(nodep);
-        ++m_ctx.m_nonRepLhs;
+        convertEquation(nodep, nodep->lhsp(), nodep->rhsp());
     }
 
     void visit(AstVarRef* nodep) override {
@@ -259,6 +308,71 @@ class AstToDfgVisitor final : public VNVisitor {
         // Build the DFG
         iterateChildren(&module);
         UASSERT_OBJ(m_uncommittedVertices.empty(), &module, "Uncommitted vertices remain");
+
+        // Canonicalize variable assignments
+        for (DfgVar* const varp : m_varps) {
+            // Gather (and unlink) all drivers
+            struct Driver {
+                FileLine* flp;
+                uint32_t lsb;
+                DfgVertex* vtxp;
+                Driver(FileLine* flp, uint32_t lsb, DfgVertex* vtxp)
+                    : flp{flp}
+                    , lsb{lsb}
+                    , vtxp{vtxp} {}
+            };
+            std::vector<Driver> drivers;
+            drivers.reserve(varp->arity());
+            varp->forEachSourceEdge([varp, &drivers](DfgEdge& edge, size_t idx) {
+                UASSERT(edge.sourcep(), "Should not have created undriven sources");
+                drivers.emplace_back(varp->driverFileLine(idx), varp->driverLsb(idx),
+                                     edge.sourcep());
+                edge.unlinkSource();
+            });
+
+            // Sort drivers by LSB
+            std::stable_sort(drivers.begin(), drivers.end(),
+                             [](const Driver& a, const Driver& b) { return a.lsb < b.lsb; });
+
+            // TODO: bail on multidriver
+
+            // Coalesce adjacent ranges
+            for (size_t i = 0, j = 1; j < drivers.size(); ++j) {
+                Driver& a = drivers[i];
+                Driver& b = drivers[j];
+
+                // Coalesce adjacent range
+                const uint32_t aWidth = a.vtxp->width();
+                const uint32_t bWidth = b.vtxp->width();
+                if (a.lsb + aWidth == b.lsb) {
+                    const auto dtypep = DfgVertex::dtypeForWidth(aWidth + bWidth);
+                    DfgConcat* const concatp = new DfgConcat{*m_dfgp, a.flp, dtypep};
+                    concatp->rhsp(a.vtxp);
+                    concatp->lhsp(b.vtxp);
+                    a.vtxp = concatp;
+                    b.vtxp = nullptr;  // Mark as moved
+                    ++m_ctx.m_coalescedAssignments;
+                    continue;
+                }
+
+                ++i;
+
+                // Compact non-adjacent ranges within the vector
+                if (j != i) {
+                    Driver& c = drivers[i];
+                    UASSERT_OBJ(!c.vtxp, c.flp, "Should have been marked moved");
+                    c = b;
+                    b.vtxp = nullptr;  // Mark as moved
+                }
+            }
+
+            // Reinsert sources in order
+            varp->resetSources();
+            for (const Driver& driver : drivers) {
+                if (!driver.vtxp) break;  // Stop at end of cmpacted list
+                varp->addDriver(driver.flp, driver.lsb, driver.vtxp);
+            }
+        }
     }
 
 public:

src/V3DfgDfgToAst.cpp

@@ -127,19 +127,21 @@ class DfgToAstVisitor final : DfgVisitor {
     // Given a DfgVar, return the canonical AstVar that can be used for this DfgVar.
     // Also builds the m_canonVars map as a side effect.
     AstVar* getCanonicalVar(const DfgVar* vtxp) {
-        // Variable only read by DFG
-        if (!vtxp->driverp()) return vtxp->varp();
+        // If variable driven (at least partially) outside the DFG, then we have no choice
+        if (!vtxp->isDrivenFullyByDfg()) return vtxp->varp();
 
         // Look up map
         const auto it = m_canonVars.find(vtxp->varp());
         if (it != m_canonVars.end()) return it->second;
 
-        // Not known yet, compute it (for all vars from the same driver)
+        // Not known yet, compute it (for all vars driven fully from the same driver)
         std::vector<const DfgVar*> varps;
-        vtxp->driverp()->forEachSink([&](const DfgVertex& vtx) {
-            if (const DfgVar* const varVtxp = vtx.cast<DfgVar>()) varps.push_back(varVtxp);
+        vtxp->source(0)->forEachSink([&](const DfgVertex& vtx) {
+            if (const DfgVar* const varVtxp = vtx.cast<DfgVar>()) {
+                if (varVtxp->isDrivenFullyByDfg()) varps.push_back(varVtxp);
+            }
         });
-        UASSERT_OBJ(!varps.empty(), vtxp, "The input vtxp->varp() is always available");
+        UASSERT_OBJ(!varps.empty(), vtxp, "The input vtxp is always available");
         std::stable_sort(varps.begin(), varps.end(), [](const DfgVar* ap, const DfgVar* bp) {
             if (ap->hasExtRefs() != bp->hasExtRefs()) return ap->hasExtRefs();
             const FileLine& aFl = *(ap->fileline());
@@ -168,11 +170,13 @@ class DfgToAstVisitor final : DfgVisitor {
             if (const DfgVar* const thisDfgVarp = vtxp->cast<DfgVar>()) {
                 // This is a DfgVar
                 varp = getCanonicalVar(thisDfgVarp);
-            } else if (const DfgVar* const sinkDfgVarp = vtxp->findSink<DfgVar>()) {
-                // We found a DfgVar driven by this node
+            } else if (const DfgVar* const sinkDfgVarp = vtxp->findSink<DfgVar>(
+                           [](const DfgVar& var) { return var.isDrivenFullyByDfg(); })) {
+                // We found a DfgVar driven fully by this node
                 varp = getCanonicalVar(sinkDfgVarp);
             } else {
-                // No DfgVar driven by this node. Create a temporary.
+                // No DfgVar driven fully by this node. Create a temporary.
+                // TODO: should we reuse parts when the AstVar is used as an rvalue?
                 const string name = m_tmpNames.get(vtxp->hash(m_hashCache).toString());
                 // Note: It is ok for these temporary variables to be always unsigned. They are
                 // read only by other expressions within the graph and all expressions interpret
@@ -208,6 +212,62 @@ class DfgToAstVisitor final : DfgVisitor {
         }
     }
 
+    void convertCanonicalVarDriver(const DfgVar* dfgVarp) {
+        const auto wRef = [dfgVarp]() {
+            return new AstVarRef{dfgVarp->fileline(), dfgVarp->varp(), VAccess::WRITE};
+        };
+        if (dfgVarp->isDrivenFullyByDfg()) {
+            // Whole variable is driven. Render driver and assign directly to whole variable.
+            AstNodeMath* const rhsp = convertDfgVertexToAstNodeMath(dfgVarp->source(0));
+            addResultEquation(dfgVarp->driverFileLine(0), wRef(), rhsp);
+        } else {
+            // Variable is driven partially. Render each driver as a separate assignment.
+            dfgVarp->forEachSourceEdge([&](const DfgEdge& edge, size_t idx) {
+                UASSERT_OBJ(edge.sourcep(), dfgVarp, "Should have removed undriven sources");
+                // Render the rhs expression
+                AstNodeMath* const rhsp = convertDfgVertexToAstNodeMath(edge.sourcep());
+                // Create select LValue
+                FileLine* const flp = dfgVarp->driverFileLine(idx);
+                AstConst* const lsbp = new AstConst{flp, dfgVarp->driverLsb(idx)};
+                AstConst* const widthp = new AstConst{flp, edge.sourcep()->width()};
+                AstSel* const lhsp = new AstSel{flp, wRef(), lsbp, widthp};
+                // Add assignment of the value to the selected bits
+                addResultEquation(flp, lhsp, rhsp);
+            });
+        }
+    }
+
+    void convertDuplicateVarDriver(const DfgVar* dfgVarp, AstVar* canonVarp) {
+        const auto rRef = [canonVarp]() {
+            return new AstVarRef{canonVarp->fileline(), canonVarp, VAccess::READ};
+        };
+        const auto wRef = [dfgVarp]() {
+            return new AstVarRef{dfgVarp->fileline(), dfgVarp->varp(), VAccess::WRITE};
+        };
+        if (dfgVarp->isDrivenFullyByDfg()) {
+            // Whole variable is driven. Just assign from the canonical variable.
+            addResultEquation(dfgVarp->driverFileLine(0), wRef(), rRef());
+        } else {
+            // Variable is driven partially. Asign from parts of the canonical var.
+            dfgVarp->forEachSourceEdge([&](const DfgEdge& edge, size_t idx) {
+                UASSERT_OBJ(edge.sourcep(), dfgVarp, "Should have removed undriven sources");
+                // Create select LValue
+                FileLine* const flp = dfgVarp->driverFileLine(idx);
+                AstConst* const lsbp = new AstConst{flp, dfgVarp->driverLsb(idx)};
+                AstConst* const widthp = new AstConst{flp, edge.sourcep()->width()};
+                AstSel* const rhsp = new AstSel{flp, rRef(), lsbp, widthp->cloneTree(false)};
+                AstSel* const lhsp = new AstSel{flp, wRef(), lsbp->cloneTree(false), widthp};
+                // Add assignment of the value to the selected bits
+                addResultEquation(flp, lhsp, rhsp);
+            });
+        }
+    }
+
+    void addResultEquation(FileLine* flp, AstNode* lhsp, AstNode* rhsp) {
+        m_modp->addStmtsp(new AstAssignW{flp, lhsp, rhsp});
+        ++m_ctx.m_resultEquations;
+    }
+
     // VISITORS
     void visit(DfgVertex* vtxp) override {  // LCOV_EXCL_START
         vtxp->v3fatal("Unhandled DfgVertex: " << vtxp->typeName());
@@ -231,57 +291,51 @@ class DfgToAstVisitor final : DfgVisitor {
         // We can eliminate some variables completely
         std::vector<AstVar*> redundantVarps;
 
-        // Render the logic
+        // Convert vertices back to assignments
         dfg.forEachVertex([&](DfgVertex& vtx) {
-            // Compute the AstNodeMath expression representing this DfgVertex
-            AstNodeMath* rhsp = nullptr;
-            AstNodeMath* lhsp = nullptr;
-            FileLine* assignmentFlp = nullptr;
+            // Render variable assignments
             if (const DfgVar* const dfgVarp = vtx.cast<DfgVar>()) {
                 // DfgVar instances (these might be driving the given AstVar variable)
-                // If there is no driver (i.e.: this DfgVar is an input to the Dfg), then nothing
-                // to do
-                if (!dfgVarp->driverp()) return;
+                // If there is no driver (i.e.: this DfgVar is an input to the Dfg), then
+                // nothing to do
+                if (!dfgVarp->isDrivenByDfg()) return;
                 // The driver of this DfgVar might drive multiple variables. Only emit one
                 // assignment from the driver to an arbitrarily chosen canonical variable, and
                 // assign the other variables from that canonical variable
                 AstVar* const canonVarp = getCanonicalVar(dfgVarp);
                 if (canonVarp == dfgVarp->varp()) {
                     // This is the canonical variable, so render the driver
-                    rhsp = convertDfgVertexToAstNodeMath(dfgVarp->driverp());
+                    convertCanonicalVarDriver(dfgVarp);
                 } else if (dfgVarp->keep()) {
-                    // Not the canonical variable but it must be kept, just assign from the
-                    // canonical variable.
-                    rhsp = new AstVarRef{canonVarp->fileline(), canonVarp, VAccess::READ};
+                    // Not the canonical variable but it must be kept
+                    convertDuplicateVarDriver(dfgVarp, canonVarp);
                 } else {
                     // Not a canonical var, and it can be removed. We will replace all references
                     // to it with the canonical variable, and hence this can be removed.
                     redundantVarps.push_back(dfgVarp->varp());
                     ++m_ctx.m_replacedVars;
-                    return;
                 }
-                // The Lhs is the variable driven by this DfgVar
-                lhsp = new AstVarRef{vtx.fileline(), dfgVarp->varp(), VAccess::WRITE};
-                // Set location to the location of the original assignment to this variable
-                assignmentFlp = dfgVarp->assignmentFileline();
-            } else if (vtx.hasMultipleSinks() && !vtx.findSink<DfgVar>()) {
-                // DfgVertex that has multiple sinks, but does not drive a DfgVar (needs temporary)
-                // Just render the logic
-                rhsp = convertDfgVertexToAstNodeMath(&vtx);
-                // The lhs is a temporary
-                lhsp = new AstVarRef{vtx.fileline(), getResultVar(&vtx), VAccess::WRITE};
-                // Render vertex
-                assignmentFlp = vtx.fileline();
-                // Stats
-                ++m_ctx.m_intermediateVars;
-            } else {
-                // Every other DfgVertex will be inlined by 'convertDfgVertexToAstNodeMath' as an
-                // AstNodeMath at use, and hence need not be converted.
                 return;
             }
-            // Add assignment of the value to the variable
-            m_modp->addStmtsp(new AstAssignW{assignmentFlp, lhsp, rhsp});
-            ++m_ctx.m_resultEquations;
+
+            // Vertices driving a single vertex will be in-lined by 'convertDfgVertexToAstNodeMath'
+            if (!vtx.hasMultipleSinks()) return;
+
+            // Vertices with multiple sinks needs a temporary if they do not fully drive a DfgVar
+            const bool needsTemporary = !vtx.findSink<DfgVar>([](const DfgVar& var) {  //
+                return var.isDrivenFullyByDfg();
+            });
+            if (needsTemporary) {
+                // DfgVertex that has multiple sinks, but does not drive a DfgVar (needs temporary)
+                ++m_ctx.m_intermediateVars;
+                // Just render the logic
+                AstNodeMath* const rhsp = convertDfgVertexToAstNodeMath(&vtx);
+                // The lhs is a temporary
+                AstNodeMath* const lhsp
+                    = new AstVarRef{vtx.fileline(), getResultVar(&vtx), VAccess::WRITE};
+                // Add assignment of the value to the variable
+                addResultEquation(vtx.fileline(), lhsp, rhsp);
+            }
         });
 
         // Remap all references to point to the canonical variables, if one exists

src/V3DfgPasses.cpp

@@ -53,6 +53,7 @@ V3DfgOptimizationContext::V3DfgOptimizationContext(const std::string& label)
 V3DfgOptimizationContext::~V3DfgOptimizationContext() {
     const string prefix = "Optimizations, DFG " + m_label + " ";
     V3Stats::addStat(prefix + "General, modules", m_modules);
+    V3Stats::addStat(prefix + "Ast2Dfg, coalesced assignments", m_coalescedAssignments);
     V3Stats::addStat(prefix + "Ast2Dfg, input equations", m_inputEquations);
     V3Stats::addStat(prefix + "Ast2Dfg, representable", m_representable);
     V3Stats::addStat(prefix + "Ast2Dfg, non-representable (dtype)", m_nonRepDType);
@@ -79,8 +80,9 @@ void V3DfgPasses::inlineVars(DfgGraph& dfg) {
     dfg.forEachVertex([](DfgVertex& vtx) {
         // For each DfgVar that has a known driver
         if (DfgVar* const varVtxp = vtx.cast<DfgVar>()) {
-            if (DfgVertex* const driverp = varVtxp->driverp()) {
+            if (varVtxp->isDrivenFullyByDfg()) {
                 // Make consumers of the DfgVar consume the driver directly
+                DfgVertex* const driverp = varVtxp->source(0);
                 varVtxp->forEachSinkEdge([=](DfgEdge& edge) { edge.relinkSource(driverp); });
             }
         }
@@ -123,7 +125,10 @@ void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) {
         if (varp->hasSinks()) return;
 
         // Can't remove if read in the module and driven here (i.e.: it's an output of the DFG)
-        if (varp->hasModRefs() && varp->driverp()) return;
+        if (varp->hasModRefs() && varp->isDrivenByDfg()) return;
+
+        // Can't remove if only partially driven by the DFG
+        if (varp->isDrivenByDfg() && !varp->isDrivenFullyByDfg()) return;
 
         // Can't remove if referenced externally, or other special reasons
         if (varp->keep()) return;
@@ -131,7 +136,8 @@ void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) {
         // If the driver of this variable has multiple non-variable sinks, then we would need
         // a temporary when rendering the graph. Instead of introducing a temporary, keep the
         // first variable that is driven by that driver
-        if (DfgVertex* const driverp = varp->driverp()) {
+        if (varp->isDrivenByDfg()) {
+            DfgVertex* const driverp = varp->source(0);
             unsigned nonVarSinks = 0;
             const DfgVar* firstSinkVarp = nullptr;
             const bool keepFirst = driverp->findSink<DfgVertex>([&](const DfgVertex& sink) {
@@ -147,7 +153,7 @@ void V3DfgPasses::removeVars(DfgGraph& dfg, DfgRemoveVarsContext& ctx) {
             if (keepFirst && firstSinkVarp == varp) return;
         }
 
-        // OK, we can delete this DfgVar!
+        // OK, we can delete this DfgVar
         ++ctx.m_removed;
 
         // If not referenced outside the DFG, then also delete the referenced AstVar,
@@ -177,11 +183,13 @@ void V3DfgPasses::optimize(DfgGraph& dfg, V3DfgOptimizationContext& ctx) {
     // There is absolutely nothing useful we can do with a graph of size 2 or less
     if (dfg.size() <= 2) return;
 
-    // We consider a DFG trivial if it contains no more than 1 non-variable, non-constant vertex
+    // We consider a DFG trivial if it contains no more than 1 non-variable, non-constant vertex,
+    // or if if it contains a DfgConcat, which can be introduced through assinment coalescing.
     unsigned excitingVertices = 0;
     const bool isTrivial = !dfg.findVertex<DfgVertex>([&](const DfgVertex& vtx) {  //
         if (vtx.is<DfgVar>()) return false;
         if (vtx.is<DfgConst>()) return false;
+        if (vtx.is<DfgConcat>()) return true;
         return ++excitingVertices >= 2;
     });
 

src/V3DfgPasses.h

@@ -54,6 +54,7 @@ class V3DfgOptimizationContext final {
 
 public:
     VDouble0 m_modules;  // Number of modules optimized
+    VDouble0 m_coalescedAssignments;  // Number of partial assignments coalesced
     VDouble0 m_inputEquations;  // Number of input combinational equations
     VDouble0 m_representable;  // Number of combinational equations representable
     VDouble0 m_nonRepDType;  // Equations non-representable due to data type