Fix select assignment expansion (#6757)

src/V3Broken.cpp

@@ -158,6 +158,10 @@ class BrokenCheckVisitor final : public VNVisitorConst {
     std::map<const AstVar*, const AstNodeVarRef*> m_suspectRefs;
     // Local variables declared in the scope of the current statement
     std::vector<std::unordered_set<const AstVar*>> m_localsStack;
+    // Number of write references encountered
+    size_t m_nWriteRefs = 0;
+    // Number of function calls encountered
+    size_t m_nCalls = 0;
 
     // STATE - for current visit position (use VL_RESTORER)
     const AstCFunc* m_cfuncp = nullptr;  // Current CFunc, if any
@@ -225,7 +229,17 @@ private:
     }
     // VISITORS
     void visit(AstNodeAssign* nodep) override {
-        processAndIterate(nodep);
+        processEnter(nodep);
+        iterateConst(nodep->rhsp());
+        const size_t nWriteRefs = m_nWriteRefs;
+        const size_t nCalls = m_nCalls;
+        iterateConst(nodep->lhsp());
+        // TODO: Enable this when #6756 is fixed
+        // Only check if there are no calls on the LHS, as calls might return an LValue
+        if (false && v3Global.assertDTypesResolved() && m_nCalls == nCalls) {
+            UASSERT_OBJ(m_nWriteRefs > nWriteRefs, nodep, "No write refs on LHS of assignment");
+        }
+        processExit(nodep);
         UASSERT_OBJ(!(v3Global.assertDTypesResolved() && VN_IS(nodep->lhsp(), NodeVarRef)
                       && !VN_AS(nodep->lhsp(), NodeVarRef)->access().isWriteOrRW()),
                     nodep, "Assignment LHS is not an lvalue");
@@ -270,6 +284,19 @@ private:
                 }
             }
         }
+        if (nodep->access().isWriteOrRW()) ++m_nWriteRefs;
+    }
+    void visit(AstNodeCCall* nodep) override {
+        ++m_nCalls;
+        processAndIterate(nodep);
+    }
+    void visit(AstCMethodHard* nodep) override {
+        ++m_nCalls;
+        processAndIterate(nodep);
+    }
+    void visit(AstNodeFTaskRef* nodep) override {
+        ++m_nCalls;
+        processAndIterate(nodep);
     }
     void visit(AstCFunc* nodep) override {
         UASSERT_OBJ(!m_cfuncp, nodep, "Nested AstCFunc");

src/V3Expand.cpp

@@ -728,7 +728,8 @@ class ExpandVisitor final : public VNVisitor {
                 UINFO(8, "    ASSIGNSEL(varlsb,wide,1bit) " << nodep);
                 AstNodeExpr* const rhsp = nodep->rhsp()->unlinkFrBack();
                 AstNodeExpr* const destp = lhsp->fromp()->unlinkFrBack();
-                AstNodeExpr* oldvalp = newWordSelBit(lfl, destp, lhsp->lsbp());
+                AstNodeExpr* oldvalp
+                    = newWordSelBit(lfl, destp->cloneTreePure(false), lhsp->lsbp());
                 fixCloneLvalue(oldvalp);
                 if (!ones) {
                     oldvalp = new AstAnd{

src/V3Inline.cpp

@@ -566,6 +566,7 @@ void inlineCell(AstNodeModule* modp, AstCell* cellp, bool last) {
 
         // Warn
         V3Inst::checkOutputShort(pinp);
+        if (!pinp->exprp()) continue;
 
         // Pick up the old and new port variables signal (new is the same on last instance)
         const AstVar* const oldModVarp = pinp->modVarp();

src/V3Inst.cpp

@@ -58,9 +58,10 @@ class InstVisitor final : public VNVisitor {
             // Simplify it
             V3Inst::pinReconnectSimple(nodep, m_cellp, false);
         }
-        if (!nodep->exprp()) return;  // No-connect
         UINFOTREE(9, nodep, "", "Pin_oldb");
+        if (!nodep->exprp()) return;  // No-connect
         V3Inst::checkOutputShort(nodep);
+        if (!nodep->exprp()) return;  // Connection removed by checkOutputShort
         // Use user1p on the PIN to indicate we created an assign for this pin
         if (!nodep->user1SetOnce()) {
             // Make an ASSIGNW (expr, pin)
@@ -644,6 +645,7 @@ public:
             // Make a new temp wire
             // UINFOTREE(9, pinp, "", "in_pin");
             V3Inst::checkOutputShort(pinp);
+            if (!pinp->exprp()) return nullptr;
             // Simplify, so stuff like '{a[0], b[0]}[1]' produced during
             // instance array expansion are brought to normal 'a[0]'
             AstNodeExpr* const pinexprp
@@ -700,6 +702,8 @@ void V3Inst::checkOutputShort(const AstPin* nodep) {
             // Uses v3warn for error, as might be found multiple times
             nodep->v3warn(E_PORTSHORT, "Output port is connected to a constant pin,"
                                        " electrical short");
+            // Delete so we don't create a 'CONST = ...' assignment
+            nodep->exprp()->unlinkFrBack()->deleteTree();
         }
     }
 }

src/V3Randomize.cpp

@@ -2174,6 +2174,8 @@ class RandomizeVisitor final : public VNVisitor {
     AstNodeExpr* makeSiblingRefp(AstNodeExpr* const exprp, AstVar* const varp,
                                  const VAccess access) {
         if (AstMemberSel* const memberSelp = VN_CAST(exprp, MemberSel)) {
+            // TODO: this ignored 'access' and will create a read reference in
+            // t_randomize_inline_var_ctl, see issue #6756
             return new AstMemberSel{exprp->fileline(), memberSelp->fromp()->cloneTree(false),
                                     varp};
         }