Remove assignments with strengths weaker than strongest non-tristate RHS (#3629)

src/V3Tristate.cpp

@@ -57,11 +57,16 @@
 //
 //
 // Another thing done in this phase is signal strength handling.
-// Currently they are only supported in assignments and only in case when the strongest assignment
+// Currently they are only supported in assignments and gates parsed as assignments (see verilog.y)
-// has constant with all bits equal on the RHS. It is the case when they can be statically
+// and only in case when the strongest assignment has RHS marked as non-tristate. It is the case
-// resolved.
+// when they can be statically resolved.
+// If the RHS is equal to z, that assignment has to be skipped. Since the value may be not known at
+// verilation time, cases with tristates on RHS can't be handled statically.
 //
-// Static resolution is done in the following way:
+// Static resolution is split into 2 parts.
+// First part can be done before tristate propagation. It is about removing assignments that are
+// weaker or equally strong as the strongest assignment with constant on RHS that has all bits
+// the same (equal to 0 or 1). It is done in the following way:
 // - The assignment of value 0 (size may be greater than 1), that has greatest strength (the
 // one corresponding to 0) of all other assignments of 0, has to be found.
 // - The same is done for value '1 and strength corresponding to value 1.
@@ -72,16 +77,19 @@
 // both strengths non-greater that the one was found, because they are weaker no matter what is on
 // RHS.
 //
-// All assignments that are stronger than the one with strongest constant are left as they are.
+// Second part of static resolution is done after tristate propagation.
+// At that moment it is known that some expressions can't be equal to z. The exact value is
+// unknown (except the ones with constants that were handled before), so weaker of both strengths
+// has to be taken into account. All weaker assignments can be safely removed. It is done in
+// similar way to the first part:
+// - The assignment with non-tristate RHS with the greatest weaker strength has to be found.
+// - Then all not stronger assignments can be removed.
+//
+// All assignments that are stronger than the strongest with non-tristate RHS are left as they are.
 //
 // There is a possible problem with equally strong assignments, because multiple assignments with
 // the same strength, but different values should result in x value, but these values are
 // unsupported.
-//
-// Singal strength can also be used in simple logic gates parsed as assignments (see verilog.y),
-// but these gates are then either removed (if they are weaker than the strongest constant) or
-// handled as the gates witout signal strengths are handled now. In other words, gate with greater
-// strength won't properly overwrite weaker driver.
 //*************************************************************************
 
 #include "config_build.h"
@@ -269,6 +277,18 @@ public:
     void associate(AstNode* fromp, AstNode* top) {
         new V3GraphEdge(&m_graph, makeVertex(fromp), makeVertex(top), 1);
     }
+    void deleteVerticesFromSubtreeRecurse(AstNode* nodep) {
+        if (!nodep) return;
+        // Skip vars, because they may be connected to more than one varref
+        if (!VN_IS(nodep, Var)) {
+            TristateVertex* const vertexp = reinterpret_cast<TristateVertex*>(nodep->user5p());
+            if (vertexp) vertexp->unlinkDelete(&m_graph);
+        }
+        deleteVerticesFromSubtreeRecurse(nodep->op1p());
+        deleteVerticesFromSubtreeRecurse(nodep->op2p());
+        deleteVerticesFromSubtreeRecurse(nodep->op3p());
+        deleteVerticesFromSubtreeRecurse(nodep->op4p());
+    }
     void setTristate(AstNode* nodep) { makeVertex(nodep)->isTristate(true); }
     bool isTristate(AstNode* nodep) {
         const TristateVertex* const vertexp = reinterpret_cast<TristateVertex*>(nodep->user5p());
@@ -715,64 +735,93 @@ class TristateVisitor final : public TristateBaseVisitor {
         return assignmentOfValueOnAllBits(*maxIt, value) ? *maxIt : nullptr;
     }
 
-    void removeWeakerAssignments(Assigns& assigns) {
+    bool isAssignmentNotStrongerThanStrength(AstAssignW* assignp, uint8_t strength) {
+        // If the value of the RHS is known and has all bits equal, only strength corresponding to
+        // its value is taken into account. In opposite case, both strengths are compared.
+        const uint8_t strength0 = getStrength(assignp, 0);
+        const uint8_t strength1 = getStrength(assignp, 1);
+        return (strength0 <= strength && strength1 <= strength)
+               || (strength0 <= strength && assignmentOfValueOnAllBits(assignp, 0))
+               || (strength1 <= strength && assignmentOfValueOnAllBits(assignp, 1));
+    }
+
+    void removeNotStrongerAssignments(Assigns& assigns, AstAssignW* strongestp,
+                                      uint8_t greatestKnownStrength) {
         // Weaker assignments are these assignments that can't change the final value of the net.
-        // If the value of the RHS is known, only strength corresponding to its value is taken into
+        // They can be safely removed. Assignments of the same strength are also removed, because
-        // account. Assignments of constants that have bits both 0 and 1 are skipped here, because
+        // duplicates aren't needed. One problem is with 2 assignments of different values and
-        // it would involve handling parts of bits separately.
+        // equal strengths. It should result in assignment of x value, but these values aren't
-
+        // supported now.
-        // First, the strongest assignment, that has value on the RHS consisting of only 1 or only
+        auto removedIt = std::remove_if(assigns.begin(), assigns.end(), [&](AstAssignW* assignp) {
-        // 0, has to be found.
+            if (assignp == strongestp) return false;
-        AstAssignW* const strongest0p = getStrongestAssignmentOfValue(assigns, 0);
+            if (isAssignmentNotStrongerThanStrength(assignp, greatestKnownStrength)) {
-        AstAssignW* const strongest1p = getStrongestAssignmentOfValue(assigns, 1);
+                // Vertices corresponding to nodes from removed assignment's subtree have to be
-        AstAssignW* strongestp = nullptr;
+                // removed.
-        uint8_t greatestKnownStrength = 0;
+                m_tgraph.deleteVerticesFromSubtreeRecurse(assignp);
-        const auto getIfStrongest = [&](AstAssignW* const strongestCandidatep, bool value) {
+                VL_DO_DANGLING(pushDeletep(assignp->unlinkFrBack()), assignp);
-            if (!strongestCandidatep) return;
+                return true;
-            uint8_t strength = getStrength(strongestCandidatep, value);
-            if (strength >= greatestKnownStrength) {
-                greatestKnownStrength = strength;
-                strongestp = strongestCandidatep;
             }
-        };
+            return false;
-        getIfStrongest(strongest0p, 0);
+        });
-        getIfStrongest(strongest1p, 1);
+        assigns.erase(removedIt, assigns.end());
+    }
 
-        if (strongestp) {
+    void removeAssignmentsNotStrongerThanUniformConstant() {
-            // Then all weaker assignments can be safely removed.
+        // If a stronger assignment of a constant with all bits equal to the same
-            // Assignments of the same strength are also removed, because duplicates aren't needed.
+        // value (0 or 1), is found, all weaker assignments can be safely removed.
-            // One problem is with 2 assignments of different values and equal strengths. It should
+        for (auto& varpAssigns : m_assigns) {
-            // result in assignment of x value, but these values aren't supported now.
+            Assigns& assigns = varpAssigns.second;
-            auto removedIt
+            if (assigns.size() > 1) {
-                = std::remove_if(assigns.begin(), assigns.end(), [&](AstAssignW* assignp) {
+                AstAssignW* const strongest0p = getStrongestAssignmentOfValue(assigns, 0);
-                      if (assignp == strongestp) return false;
+                AstAssignW* const strongest1p = getStrongestAssignmentOfValue(assigns, 1);
-                      const uint8_t strength0 = getStrength(assignp, 0);
+                AstAssignW* strongestp = nullptr;
-                      const uint8_t strength1 = getStrength(assignp, 1);
+                uint8_t greatestKnownStrength = 0;
-                      const bool toRemove = (strength0 <= greatestKnownStrength
+                const auto getIfStrongest
-                                             && strength1 <= greatestKnownStrength)
+                    = [&](AstAssignW* const strongestCandidatep, bool value) {
-                                            || (strength0 <= greatestKnownStrength
+                          if (!strongestCandidatep) return;
-                                                && assignmentOfValueOnAllBits(assignp, 0))
+                          uint8_t strength = getStrength(strongestCandidatep, value);
-                                            || (strength1 <= greatestKnownStrength
+                          if (strength >= greatestKnownStrength) {
-                                                && assignmentOfValueOnAllBits(assignp, 1));
+                              greatestKnownStrength = strength;
-                      if (toRemove) {
+                              strongestp = strongestCandidatep;
-                          // Don't propagate tristate if its assignment is removed.
+                          }
-                          TristateVertex* const vertexp
+                      };
-                              = reinterpret_cast<TristateVertex*>(assignp->rhsp()->user5p());
+                getIfStrongest(strongest0p, 0);
-                          if (vertexp) vertexp->isTristate(false);
+                getIfStrongest(strongest1p, 1);
-                          VL_DO_DANGLING(pushDeletep(assignp->unlinkFrBack()), assignp);
+
-                          return true;
+                if (strongestp) {
-                      }
+                    removeNotStrongerAssignments(assigns, strongestp, greatestKnownStrength);
-                      return false;
+                }
-                  });
+            }
-            assigns.erase(removedIt, assigns.end());
         }
     }
 
-    void resolveMultipleNetAssignments() {
+    void removeAssignmentsNotStrongerThanNonTristate() {
+        // Similar function as removeAssignmentsNotStrongerThanUniformConstant, but here the
+        // assignments that have strength not stronger than the strongest assignment with
+        // non-tristate RHS are removed. Strengths are compared according to their smaller values,
+        // because the values of RHSs are unknown. (Assignments not stronger than strongest
+        // constant are already removed.)
         for (auto& varpAssigns : m_assigns) {
-            if (varpAssigns.second.size() > 1) {
+            Assigns& assigns = varpAssigns.second;
-                // first the static resolution is tried
+            if (assigns.size() > 1) {
-                removeWeakerAssignments(varpAssigns.second);
+                auto maxIt = std::max_element(
+                    assigns.begin(), assigns.end(), [&](AstAssignW* ap, AstAssignW* bp) {
+                        if (m_tgraph.isTristate(ap)) return !m_tgraph.isTristate(bp);
+                        if (m_tgraph.isTristate(bp)) return false;
+                        const uint8_t minStrengthA
+                            = std::min(getStrength(ap, 0), getStrength(ap, 1));
+                        const uint8_t minStrengthB
+                            = std::min(getStrength(bp, 0), getStrength(bp, 1));
+                        return minStrengthA < minStrengthB;
+                    });
+                // If RHSs of all assignments are tristate, 1st element is returned, so it is
+                // needed to check if it is non-tristate.
+                AstAssignW* const strongestp = m_tgraph.isTristate(*maxIt) ? nullptr : *maxIt;
+                if (strongestp) {
+                    uint8_t greatestKnownStrength
+                        = std::min(getStrength(strongestp, 0), getStrength(strongestp, 1));
+                    removeNotStrongerAssignments(assigns, strongestp, greatestKnownStrength);
+                }
             }
         }
     }
@@ -1534,10 +1583,14 @@ class TristateVisitor final : public TristateBaseVisitor {
                 iterateChildren(nodep);
                 m_graphing = false;
             }
-            // resolve multiple net assignments and signal strengths
+            // Remove all assignments not stronger than the strongest uniform constant
-            resolveMultipleNetAssignments();
+            removeAssignmentsNotStrongerThanUniformConstant();
             // Use graph to find tristate signals
             m_tgraph.graphWalk(nodep);
+
+            // Remove all assignments not stronger than the strongest non-tristate RHS
+            removeAssignmentsNotStrongerThanNonTristate();
+
             // Build the LHS drivers map for this module
             iterateChildren(nodep);
             // Insert new logic for all tristates

test_regress/t/t_strength_strongest_constant.v

@@ -4,15 +4,30 @@
 // any use, without warranty, 2022 by Antmicro Ltd.
 // SPDX-License-Identifier: CC0-1.0
 
-module t (/*AUTOARG*/);
+module t (clk1, clk2);
+   input wire clk1;
+   input wire clk2;
+
    wire a;
    nor (pull0, weak1) n1(a, 0, 0);
    assign (strong0, weak1) a = 0;
 
+   wire [1:0] b;
+   assign (weak0, supply1) b = '1;
+   assign b = clk1 ? '0 : 'z;
+
+   wire c = 1;
+   assign (weak0, pull1) c = clk1 & clk2;
+
    always begin
-      if (!a) begin
+      if (!a && b === '1 && c) begin
          $write("*-* All Finished *-*\n");
          $finish;
       end
+      else begin
+         $write("Error: a = %b, b = %b, c = %b ", a, b, c);
+         $write("expected: a = 0, b = 11, c = 1\n");
+         $stop;
+      end
    end
 endmodule

test_regress/t/t_strength_strongest_non_tristate.pl

@@ -0,0 +1,21 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2022 by Antmicro Ltd. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(simulator => 1);
+
+compile(
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;

test_regress/t/t_strength_strongest_non_tristate.v

@@ -0,0 +1,35 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2022 by Antmicro Ltd.
+// SPDX-License-Identifier: CC0-1.0
+
+module t (clk1, clk2);
+   input wire clk1;
+   input wire clk2;
+
+   wire (weak0, weak1) a = 0;
+   assign (strong0, supply1) a = clk1;
+   assign (pull0, pull1) a = 1;
+
+   wire b;
+   xor (strong0, strong1) (b, clk1, clk2);
+   and (weak0, pull1) (b, clk1, clk2);
+
+   wire [7:0] c;
+   assign (supply0, strong1) c = clk1 ? '1 : '0;
+   assign (weak0, supply1) c = '0;
+   assign (weak0, pull1) c = 'z;
+
+   always begin
+      if (a === clk1 && b === clk1 ^ clk2 && c[0] === clk1) begin
+         $write("*-* All Finished *-*\n");
+         $finish;
+      end
+      else begin
+         $write("Error: a = %b, b = %b, c[0] = %b, ", a, b, c[0]);
+         $write("expected: a = %b, b = %b, c[0] = %b\n", clk1, clk1 ^ clk2, clk1);
+         $stop;
+      end
+   end
+endmodule