From 8ebe86e54b94fce2d48e9b6204f44728d679e0df Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Sat, 16 Nov 2019 17:23:51 -0500
Subject: [PATCH] Fix color assertion on empty if, bug1604.

---
 Changes                            |  2 +
 src/V3Split.cpp                    | 34 ++++++++--------
 test_regress/t/t_alw_split_cond.pl | 16 ++++++++
 test_regress/t/t_alw_split_cond.v  | 63 ++++++++++++++++++++++++++++++
 4 files changed, 98 insertions(+), 17 deletions(-)
 create mode 100755 test_regress/t/t_alw_split_cond.pl
 create mode 100644 test_regress/t/t_alw_split_cond.v

diff --git a/Changes b/Changes
index e2b3389f0..c96adcac2 100644
--- a/Changes
+++ b/Changes
@@ -10,6 +10,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Add error on redefining preprocessor directives. [Piotr Binkowski]
 
+****  Fix color assertion on empty if, bug1604. [Andrew Holme]
+
 ****  Fix for loop missing initializer, bug1605. [Andrew Holme]
 
 
diff --git a/src/V3Split.cpp b/src/V3Split.cpp
index a248baf22..8cdd63194 100644
--- a/src/V3Split.cpp
+++ b/src/V3Split.cpp
@@ -110,11 +110,7 @@ protected:
     // Do not make accessor for nodep(),  It may change due to
     // reordering a lower block, but we don't repair it
     virtual string name() const {
-        if (m_nodep->name() == "") {
-            return cvtToHex(m_nodep);
-        } else {
-            return m_nodep->name();
-        }
+        return cvtToHex(m_nodep) + ' ' + m_nodep->prettyTypeName();
     }
     virtual FileLine* fileline() const { return nodep()->fileline(); }
 public:
@@ -655,18 +651,12 @@ public:
     const ColorSet& colors() const { return m_colors; }
     const ColorSet& colors(AstNodeIf* nodep) const {
         IfColorMap::const_iterator it = m_ifColors.find(nodep);
-        UASSERT_OBJ(it != m_ifColors.end(), nodep, "Unknown node in split color() map");
+        UASSERT_OBJ(it != m_ifColors.end(), nodep, "Node missing from split color() map");
         return it->second;
     }
 
-protected:
-    virtual void visit(AstNodeIf* nodep) {
-        m_ifStack.push_back(nodep);
-        iterateChildren(nodep);
-        m_ifStack.pop_back();
-    }
-
-    virtual void visit(AstNode* nodep) {
+private:
+    void trackNode(AstNode* nodep) {
         if (nodep->user3p()) {
             SplitLogicVertex* vertexp = reinterpret_cast<SplitLogicVertex*>(nodep->user3p());
             uint32_t color = vertexp->color();
@@ -679,6 +669,17 @@ protected:
                 m_ifColors[*it].insert(color);
             }
         }
+    }
+
+protected:
+    virtual void visit(AstNodeIf* nodep) {
+        m_ifStack.push_back(nodep);
+        trackNode(nodep);
+        iterateChildren(nodep);
+        m_ifStack.pop_back();
+    }
+    virtual void visit(AstNode* nodep) {
+        trackNode(nodep);
         iterateChildren(nodep);
     }
 
@@ -944,15 +945,14 @@ protected:
             }
         }
 
-        if (debug()>=9) {
-            m_graph.dumpDotFilePrefixed("splitg_nodup", false);
-        }
+        if (debug()>=9) m_graph.dumpDotFilePrefixed("splitg_nodup", false);
 
         // Weak coloring to determine what needs to remain grouped
         // in a single always. This follows all edges excluding:
         //  - those we pruned above
         //  - PostEdges, which are done later
         m_graph.weaklyConnected(&SplitEdge::followScoreboard);
+        if (debug()>=9) m_graph.dumpDotFilePrefixed("splitg_colored", false);
     }
 
     virtual void visit(AstAlways* nodep) {
diff --git a/test_regress/t/t_alw_split_cond.pl b/test_regress/t/t_alw_split_cond.pl
new file mode 100755
index 000000000..5c448de7c
--- /dev/null
+++ b/test_regress/t/t_alw_split_cond.pl
@@ -0,0 +1,16 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2003 by Wilson Snyder. 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.
+
+scenarios(simulator => 1);
+
+compile(
+    );
+
+ok(1);
+1;
diff --git a/test_regress/t/t_alw_split_cond.v b/test_regress/t/t_alw_split_cond.v
new file mode 100644
index 000000000..11361438c
--- /dev/null
+++ b/test_regress/t/t_alw_split_cond.v
@@ -0,0 +1,63 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2019 by Wilson Snyder.
+
+//bug1604
+module t (/*AUTOARG*/
+   // Outputs
+   two,
+   // Inputs
+   clk, aresetn, ten
+   );
+
+   input wire clk;
+   input wire aresetn;
+
+   input reg [9:0] ten;
+   output reg [1:0] two;
+
+   // Passes with this
+   //output reg [1:0] rx;
+   //output reg [1:0] ry;
+
+   function [1:0] func
+     (
+      input [1:0] p0_x,
+      input [1:0] p0_y,
+      input [1:0] p1_x,
+      input [1:0] p1_y,
+      input [1:0] sel);
+
+      reg [1:0]   rx;
+      reg [1:0]   ry;
+
+`ifdef NOT_DEF
+      // This way works
+      rx = sel == 2'b10 ? p1_x : p0_x;
+      ry = sel == 2'b10 ? p1_y : p0_y;
+`else
+      // This way fails to compile
+      if (sel == 2'b10) begin
+         rx = p1_x;
+         ry = p1_y;
+      end
+      else begin
+         rx = p0_x;
+         ry = p0_y;
+      end
+`endif
+      // Note rx and ry are unused
+      //func = rx | ry;  // Also passes
+      func = 0;
+   endfunction
+
+   always @(*) begin
+      two = func(
+                 ten[8 +: 2],
+                 ten[6 +: 2],
+                 ten[4 +: 2],
+                 ten[2 +: 2],
+                 ten[0 +: 2]);
+   end
+endmodule