[slang-netlist] Fix connectivity of procedural conditional statements (…

…#781)

Fixes #779.

tools/netlist/README.md

@@ -1,6 +1,12 @@
 slang-netlist
 =============
 
+> **Warning**
+>
+> slang-netlist is a work in progress and may not work as expected. Check
+> TODO.md for a list of some new features and fixes that are planned. If you
+> encounter a problem, please submit a bug report via Issues.
+
 slang-netlist is a library and tool for analysing the source-level static
 connectivity of a design. This capability can be useful, for example, to
 develop structural checks or to investigate timing paths, rather than having to

tools/netlist/TODO.md

@@ -1,6 +1,42 @@
 To dos
 ======
 
+- Support for more procedural statements, the full list is:
+
+    InvalidStatement
+    EmptyStatement
+    StatementList
+    BlockStatement
+    ReturnStatement
+    BreakStatement
+    ContinueStatement
+    DisableStatement
+    VariableDeclStatement
+    ConditionalStatement
+    CaseStatement
+    PatternCaseStatement
+    ForLoopStatement
+    RepeatLoopStatement
+    ForeachLoopStatement
+    WhileLoopStatement
+    DoWhileLoopStatement
+    ForeverLoopStatement
+    ExpressionStatement
+    TimedStatement
+    ImmediateAssertionStatement
+    ConcurrentAssertionStatement
+    DisableForkStatement
+    WaitStatement
+    WaitForkStatement
+    WaitOrderStatement
+    EventTriggerStatement
+    ProceduralAssignStatement
+    ProceduralDeassignStatement
+    RandCaseStatement
+    RandSequenceStatement
+    ProceduralCheckerStatement
+
+- Dumping of a dot file outputs random characters at the end.
 - Reporting of variables in the netlist (by type, matching patterns).
 - Infer sequential elements in the netlist (ie non-blocking assignment and
   sensitive to a clock edge).

tools/netlist/include/NetlistVisitor.h

@@ -62,14 +62,14 @@ static void connectVarToVar(Netlist& netlist, NetlistNode& sourceVarNode,
 /// expressions, adding them to a visit list during the traversal.
 class VariableReferenceVisitor : public ast::ASTVisitor<VariableReferenceVisitor, false, true> {
 public:
-    explicit VariableReferenceVisitor(Netlist& netlist, std::vector<NetlistNode*>& visitList,
-                                      ast::EvalContext& evalCtx, bool leftOperand) :
+    explicit VariableReferenceVisitor(Netlist& netlist, ast::EvalContext& evalCtx,
+                                      bool leftOperand = false) :
         netlist(netlist),
-        visitList(visitList), evalCtx(evalCtx), leftOperand(leftOperand) {}
+        evalCtx(evalCtx), leftOperand(leftOperand) {}
 
     void handle(const ast::NamedValueExpression& expr) {
         auto& node = netlist.addVariableReference(expr.symbol, expr, leftOperand);
-        visitList.push_back(&node);
+        varList.push_back(&node);
         for (auto* selector : selectors) {
             if (selector->kind == ast::ExpressionKind::ElementSelect) {
                 auto index = selector->as<ast::ElementSelectExpression>().selector().eval(evalCtx);
@@ -103,53 +103,57 @@ class VariableReferenceVisitor : public ast::ASTVisitor<VariableReferenceVisitor
         expr.value().visit(*this);
     }
 
+    std::vector<NetlistNode*>& getVars() { return varList; }
+
 private:
     Netlist& netlist;
-    std::vector<NetlistNode*>& visitList;
     ast::EvalContext& evalCtx;
     /// Whether this traversal is the target of an assignment or not.
     bool leftOperand;
+    std::vector<NetlistNode*> varList;
     std::vector<const ast::Expression*> selectors;
 };
 
 /// An AST visitor to create dependencies between occurrances of variables
 /// appearing on the left and right hand sides of assignment statements.
 class AssignmentVisitor : public ast::ASTVisitor<AssignmentVisitor, false, true> {
 public:
-    explicit AssignmentVisitor(Netlist& netlist, ast::EvalContext& evalCtx) :
-        netlist(netlist), evalCtx(evalCtx) {}
+    explicit AssignmentVisitor(Netlist& netlist, ast::EvalContext& evalCtx,
+                               SmallVector<NetlistNode*>& condVars) :
+        netlist(netlist),
+        evalCtx(evalCtx), condVars(condVars) {}
 
     void handle(const ast::AssignmentExpression& expr) {
         // Collect variable references on the left-hand side of the assignment.
-        std::vector<NetlistNode*> visitListLHS, visitListRHS;
-        {
-            VariableReferenceVisitor visitor(netlist, visitListLHS, evalCtx, true);
-            expr.left().visit(visitor);
-        }
+        VariableReferenceVisitor visitorLHS(netlist, evalCtx, true);
+        expr.left().visit(visitorLHS);
         // Collect variable references on the right-hand side of the assignment.
-        {
-            VariableReferenceVisitor visitor(netlist, visitListRHS, evalCtx, false);
-            expr.right().visit(visitor);
-        }
-        // Add edge from LHS variable refrence to variable declaration.
-        for (auto* leftNode : visitListLHS) {
+        VariableReferenceVisitor visitorRHS(netlist, evalCtx, false);
+        expr.right().visit(visitorRHS);
+        for (auto* leftNode : visitorLHS.getVars()) {
+            // Add edge from LHS variable refrence to variable declaration.
             connectVarToDecl(netlist, *leftNode, getSymbolHierPath(leftNode->symbol));
-        }
-        // Add edge from variable declaration to RHS variable reference.
-        for (auto* rightNode : visitListRHS) {
-            connectDeclToVar(netlist, *rightNode, getSymbolHierPath(rightNode->symbol));
-        }
-        // Add edges form RHS expression terms to LHS expression terms.
-        for (auto* leftNode : visitListLHS) {
-            for (auto* rightNode : visitListRHS) {
+            for (auto* rightNode : visitorRHS.getVars()) {
+                // Add edge from variable declaration to RHS variable reference.
+                connectDeclToVar(netlist, *rightNode, getSymbolHierPath(rightNode->symbol));
+                // Add edge form RHS expression term to LHS expression terms.
                 connectVarToVar(netlist, *rightNode, *leftNode);
             }
         }
+        for (auto* condNode : condVars) {
+            // Add edge from conditional variable declaraiton to the reference.
+            connectDeclToVar(netlist, *condNode, getSymbolHierPath(condNode->symbol));
+            for (auto* leftNode : visitorLHS.getVars()) {
+                // Add edge from conditional variable to the LHS variable.
+                connectVarToVar(netlist, *condNode, *leftNode);
+            }
+        }
     }
 
 private:
     Netlist& netlist;
     ast::EvalContext& evalCtx;
+    SmallVector<NetlistNode*>& condVars;
 };
 
 /// An AST visitor for proceural blocks that performs loop unrolling.
@@ -230,18 +234,45 @@ class ProceduralBlockVisitor : public ast::ASTVisitor<ProceduralBlockVisitor, tr
             }
 
             loop.body.visit(*this);
-            if (anyErrors)
+            if (anyErrors) {
                 return;
+            }
         }
     }
 
     void handle(const ast::ConditionalStatement& stmt) {
-        // Evaluate the condition; if not constant visit both sides,
-        // otherwise visit only the side that matches the condition.
+        // Evaluate the condition; if not constant visit both sides (the
+        // fallback option), otherwise visit only the side that matches the
+        // condition.
+
         auto fallback = [&] {
+            // Create a list of variables appearing in the condition
+            // expression.
+            VariableReferenceVisitor varRefVisitor(netlist, evalCtx);
+            for (auto& cond : stmt.conditions) {
+                if (cond.pattern) {
+                    // Skip.
+                    continue;
+                }
+                cond.expr->visit(varRefVisitor);
+            }
+
+            // Push the condition variables.
+            for (auto& varRef : varRefVisitor.getVars()) {
+                condVarsStack.push_back(varRef);
+            }
+
+            // Visit the 'then' and 'else' statements, whose execution is
+            // under the control of the condition variables.
             stmt.ifTrue.visit(*this);
-            if (stmt.ifFalse)
+            if (stmt.ifFalse) {
                 stmt.ifFalse->visit(*this);
+            }
+
+            // Pop the conditon variables.
+            for (auto& varRef : varRefVisitor.getVars()) {
+                condVarsStack.pop_back();
+            }
         };
 
         for (auto& cond : stmt.conditions) {
@@ -257,8 +288,9 @@ class ProceduralBlockVisitor : public ast::ASTVisitor<ProceduralBlockVisitor, tr
             }
 
             if (!result.isTrue()) {
-                if (stmt.ifFalse)
+                if (stmt.ifFalse) {
                     stmt.ifFalse->visit(*this);
+                }
                 return;
             }
         }
@@ -268,7 +300,7 @@ class ProceduralBlockVisitor : public ast::ASTVisitor<ProceduralBlockVisitor, tr
 
     void handle(const ast::ExpressionStatement& stmt) {
         step();
-        AssignmentVisitor visitor(netlist, evalCtx);
+        AssignmentVisitor visitor(netlist, evalCtx, condVarsStack);
         stmt.visit(visitor);
     }
 
@@ -283,6 +315,7 @@ class ProceduralBlockVisitor : public ast::ASTVisitor<ProceduralBlockVisitor, tr
 
     Netlist& netlist;
     ast::EvalContext evalCtx;
+    SmallVector<NetlistNode*> condVarsStack;
 };
 
 /// A visitor that traverses the AST and builds a netlist representation.
@@ -347,14 +380,13 @@ class NetlistVisitor : public ast::ASTVisitor<NetlistVisitor, true, false> {
         // Handle connections to the ports of the instance.
         for (auto* portConnection : symbol.getPortConnections()) {
             // Collect variable references in the port expression.
-            std::vector<NetlistNode*> exprVisitList;
             ast::EvalContext evalCtx(compilation);
             auto portDirection = portConnection->port.as<ast::PortSymbol>().direction;
             // The port is effectively the target of an assignment if it is an
             // input.
             bool isLeftOperand = portDirection == ast::ArgumentDirection::In ||
                                  portDirection == ast::ArgumentDirection::InOut;
-            VariableReferenceVisitor visitor(netlist, exprVisitList, evalCtx, isLeftOperand);
+            VariableReferenceVisitor visitor(netlist, evalCtx, isLeftOperand);
             portConnection->getExpression()->visit(visitor);
             // Given a port hookup of the form:
             //   .foo(expr(x, y))
@@ -367,7 +399,7 @@ class NetlistVisitor : public ast::ASTVisitor<NetlistVisitor, true, false> {
             // InOut port:
             //   var decl x -> var ref x -> port var ref foo
             //   var decl y <- var ref y <- port var ref foo
-            for (auto* node : exprVisitList) {
+            for (auto* node : visitor.getVars()) {
                 switch (portDirection) {
                     case ast::ArgumentDirection::In:
                         connectDeclToVar(netlist, *node, getSymbolHierPath(node->symbol));
@@ -400,7 +432,8 @@ class NetlistVisitor : public ast::ASTVisitor<NetlistVisitor, true, false> {
     /// Continuous assignment statement.
     void handle(const ast::ContinuousAssignSymbol& symbol) {
         ast::EvalContext evalCtx(compilation);
-        AssignmentVisitor visitor(netlist, evalCtx);
+        SmallVector<NetlistNode*> condVars;
+        AssignmentVisitor visitor(netlist, evalCtx, condVars);
         symbol.visit(visitor);
     }
 

tools/netlist/tests/NetlistTests.cpp

@@ -410,3 +410,57 @@ endmodule
     CHECK(pathFinder.find(*inPortA, *outPortB).empty());
     CHECK(pathFinder.find(*inPortB, *outPortA).empty());
 }
+
+//===---------------------------------------------------------------------===//
+// Tests for conditional variables in procedural blocks.
+//===---------------------------------------------------------------------===//
+
+TEST_CASE("Mux") {
+    // Test that the variable in a conditional block is correctly added as a
+    // dependency on the output variable controlled by that block.
+    auto tree = SyntaxTree::fromText(R"(
+module mux(input a, input b, input sel, output reg f);
+  always @(*) begin
+    if (sel == 1'b0) begin
+      f = a;
+    end else begin
+      f = b;
+    end
+  end
+endmodule
+)");
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+    NO_COMPILATION_ERRORS;
+    auto netlist = createNetlist(compilation);
+    PathFinder pathFinder(netlist);
+    CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel"), *netlist.lookupPort("mux.f")).empty());
+}
+
+TEST_CASE("Nested muxing") {
+    // Test that the variables in multiple nested levels of conditions are
+    // correctly added as dependencies of the output variable.
+    auto tree = SyntaxTree::fromText(R"(
+module mux(input a, input b, input c,
+           input sel_a, input sel_b,
+           output reg f);
+  always @(*) begin
+    if (sel_a == 1'b0) begin
+      if (sel_b == 1'b0)
+        f = a;
+      else
+        f = b;
+    end else begin
+      f = c;
+    end
+  end
+endmodule
+)");
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+    NO_COMPILATION_ERRORS;
+    auto netlist = createNetlist(compilation);
+    PathFinder pathFinder(netlist);
+    CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel_a"), *netlist.lookupPort("mux.f")).empty());
+    CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel_b"), *netlist.lookupPort("mux.f")).empty());
+}