From b8842f7e55e186e15939215c73fca839f67b0d24 Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Fri, 22 Jun 2018 19:46:27 -0400
Subject: [PATCH] Fix clocker attributes to not propagate on concats.

---
 Changes                            |   2 +
 src/V3Order.cpp                    |  16 ++-
 test_regress/t/t_clocker.out       | 174 +++++++++++++++++++++++++++++
 test_regress/t/t_clocker.pl        |   4 +-
 test_regress/t/t_order_clkinst.out |  76 +++++++++++++
 test_regress/t/t_order_clkinst.pl  |   4 +-
 6 files changed, 265 insertions(+), 11 deletions(-)
 create mode 100644 test_regress/t/t_clocker.out
 create mode 100644 test_regress/t/t_order_clkinst.out

diff --git a/Changes b/Changes
index aa185079a..f44ef0621 100644
--- a/Changes
+++ b/Changes
@@ -16,6 +16,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Fix Verilation performance issues, bug1316. [John Coiner]
 
+****  Fix clocker attributes to not propagate on concats. [John Coiner]
+
 
 * Verilator 3.925 devel
 
diff --git a/src/V3Order.cpp b/src/V3Order.cpp
index 329db14c3..6d1fa87a9 100644
--- a/src/V3Order.cpp
+++ b/src/V3Order.cpp
@@ -379,27 +379,26 @@ public:
 class OrderClkAssVisitor : public AstNVisitor {
 private:
     bool m_clkAss; 	// There is signals marked as clocker in the assignment
-
     // METHODS
     VL_DEBUG_FUNC;  // Declare debug()
     virtual void visit(AstNodeAssign* nodep) {
-        if (const AstVarRef* varrefp = VN_CAST(nodep->lhsp(), VarRef) )
+        if (const AstVarRef* varrefp = VN_CAST(nodep->lhsp(), VarRef)) {
 	    if (varrefp->varp()->attrClocker() == AstVarAttrClocker::CLOCKER_YES) {
 		m_clkAss = true;
 		UINFO(6, "node was marked as clocker "<<varrefp<<endl);
 	    }
+        }
         iterateChildren(nodep->rhsp());
     }
     virtual void visit(AstVarRef* nodep) {
-	if (nodep->varp()->attrClocker() == AstVarAttrClocker::CLOCKER_YES) {
-	    m_clkAss = true;
-	    UINFO(6, "node was marked as clocker "<<nodep<<endl);
-	}
+        // Previous versions checked attrClocker() here, but this breaks
+        // the updated t_clocker VCD test.
+        // If reenable this visitor note AstNodeMath short circuit below
     }
+    virtual void visit(AstNodeMath* nodep) {} // Accelerate
     virtual void visit(AstNode* nodep) {
         iterateChildren(nodep);
     }
-
 public:
     // CONSTUCTORS
     explicit OrderClkAssVisitor(AstNode* nodep) {
@@ -407,9 +406,8 @@ public:
         iterate(nodep);
     }
     virtual ~OrderClkAssVisitor() {}
-
     // METHODS
-    bool isClkAss() {return m_clkAss;}
+    bool isClkAss() { return m_clkAss; }
 };
 
 
diff --git a/test_regress/t/t_clocker.out b/test_regress/t/t_clocker.out
new file mode 100644
index 000000000..62637a38f
--- /dev/null
+++ b/test_regress/t/t_clocker.out
@@ -0,0 +1,174 @@
+$version Generated by VerilatedVcd $end
+$date Fri Jun 22 19:23:24 2018
+ $end
+$timescale   1ns $end
+
+ $scope module top $end
+  $var wire  1 $ clk $end
+  $var wire  1 % res $end
+  $var wire 16 ' res16 [15:0] $end
+  $var wire  8 & res8 [7:0] $end
+  $scope module $unit $end
+   $var wire 32 + ID_MSB [31:0] $end
+  $upscope $end
+  $scope module t $end
+   $var wire  1 $ clk $end
+   $var wire  8 ( clkSet [7:0] $end
+   $var wire  1 $ clk_1 $end
+   $var wire  3 ) clk_3 [2:0] $end
+   $var wire  4 * clk_4 [3:0] $end
+   $var wire  1 $ clk_final $end
+   $var wire  8 # count [7:0] $end
+   $var wire  1 % res $end
+   $var wire 16 ' res16 [15:0] $end
+   $var wire  8 & res8 [7:0] $end
+  $upscope $end
+ $upscope $end
+$enddefinitions $end
+
+
+#0
+b00000000 #
+0$
+0%
+b00000000 &
+b0000000000000000 '
+b00000000 (
+b000 )
+b0000 *
+b00000000000000000000000000000001 +
+#10
+b00000001 #
+1$
+1%
+b11101111 &
+b0000000111111111 '
+b11111111 (
+b111 )
+b1111 *
+#15
+b00000010 #
+0$
+0%
+b00000000 &
+b0000001000000000 '
+b00000000 (
+b000 )
+b0000 *
+#20
+b00000011 #
+1$
+1%
+b11101111 &
+b0000001111111111 '
+b11111111 (
+b111 )
+b1111 *
+#25
+b00000100 #
+0$
+0%
+b00000000 &
+b0000010000000000 '
+b00000000 (
+b000 )
+b0000 *
+#30
+b00000101 #
+1$
+1%
+b11101111 &
+b0000010111111111 '
+b11111111 (
+b111 )
+b1111 *
+#35
+b00000110 #
+0$
+0%
+b00000000 &
+b0000011000000000 '
+b00000000 (
+b000 )
+b0000 *
+#40
+b00000111 #
+1$
+1%
+b11101111 &
+b0000011111111111 '
+b11111111 (
+b111 )
+b1111 *
+#45
+b00001000 #
+0$
+0%
+b00000000 &
+b0000100000000000 '
+b00000000 (
+b000 )
+b0000 *
+#50
+b00001001 #
+1$
+1%
+b11101111 &
+b0000100111111111 '
+b11111111 (
+b111 )
+b1111 *
+#55
+b00001010 #
+0$
+0%
+b00000000 &
+b0000101000000000 '
+b00000000 (
+b000 )
+b0000 *
+#60
+b00001011 #
+1$
+1%
+b11101111 &
+b0000101111111111 '
+b11111111 (
+b111 )
+b1111 *
+#65
+b00001100 #
+0$
+0%
+b00000000 &
+b0000110000000000 '
+b00000000 (
+b000 )
+b0000 *
+#70
+b00001101 #
+1$
+1%
+b11101111 &
+b0000110111111111 '
+b11111111 (
+b111 )
+b1111 *
+#75
+b00001110 #
+0$
+0%
+b00000000 &
+b0000111000000000 '
+b00000000 (
+b000 )
+b0000 *
+#80
+b00001111 #
+1$
+1%
+b11101111 &
+b0000111111111111 '
+b11111111 (
+b111 )
+b1111 *
diff --git a/test_regress/t/t_clocker.pl b/test_regress/t/t_clocker.pl
index 8997efa90..2335cdb17 100755
--- a/test_regress/t/t_clocker.pl
+++ b/test_regress/t/t_clocker.pl
@@ -10,12 +10,14 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
 scenarios(simulator => 1);
 
 compile(
-#    verilator_flags2 => ["-Wno-UNOPTFLAT"]
+    verilator_flags2 => ["--trace"]
     );
 
 execute(
     check_finished => 1,
     );
 
+vcd_identical("$Self->{obj_dir}/simx.vcd", "t/$Self->{name}.out");
+
 ok(1);
 1;
diff --git a/test_regress/t/t_order_clkinst.out b/test_regress/t/t_order_clkinst.out
new file mode 100644
index 000000000..a26ba0c26
--- /dev/null
+++ b/test_regress/t/t_order_clkinst.out
@@ -0,0 +1,76 @@
+$version Generated by VerilatedVcd $end
+$date Fri Jun 22 19:27:45 2018
+ $end
+$timescale   1ns $end
+
+ $scope module top $end
+  $var wire  1 / clk $end
+  $scope module t $end
+   $var wire 32 % c1_count [31:0] $end
+   $var wire  1 # c1_start $end
+   $var wire 32 ( c3_count [31:0] $end
+   $var wire  1 ' c3_start $end
+   $var wire  1 / clk $end
+   $var wire  8 $ cyc [7:0] $end
+   $var wire 32 & s2_count [31:0] $end
+   $var wire  1 # s2_start $end
+   $scope module c1 $end
+    $var wire 32 % count [31:0] $end
+    $var wire 32 * runner [31:0] $end
+    $var wire 32 ) runnerm1 [31:0] $end
+    $var wire  1 # start $end
+   $upscope $end
+   $scope module c3 $end
+    $var wire 32 ( count [31:0] $end
+    $var wire 32 . runner [31:0] $end
+    $var wire 32 - runnerm1 [31:0] $end
+    $var wire  1 ' start $end
+   $upscope $end
+   $scope module s2 $end
+    $var wire 32 & count [31:0] $end
+    $var wire 32 , runner [31:0] $end
+    $var wire 32 + runnerm1 [31:0] $end
+    $var wire  1 # start $end
+   $upscope $end
+  $upscope $end
+ $upscope $end
+$enddefinitions $end
+
+
+#0
+0#
+b00000000 $
+b00000000000000000000000000000000 %
+b00000000000000000000000000000000 &
+0'
+b00000000000000000000000000000000 (
+b11111111111111111111111111111111 )
+b00000000000000000000000000000000 *
+b11111111111111111111111111111111 +
+b00000000000000000000000000000000 ,
+b11111111111111111111111111111111 -
+b00000000000000000000000000000000 .
+0/
+#10
+b00000001 $
+1/
+#15
+0/
+#20
+1#
+b00000010 $
+b00000000000000000000000000000011 %
+b00000000000000000000000000000011 &
+1'
+b00000000000000000000000000000101 (
+1/
+#25
+0/
+#30
+b00000011 $
+1/
+#35
+0/
+#40
+b00000100 $
+1/
diff --git a/test_regress/t/t_order_clkinst.pl b/test_regress/t/t_order_clkinst.pl
index d151aacb9..4e593fe93 100755
--- a/test_regress/t/t_order_clkinst.pl
+++ b/test_regress/t/t_order_clkinst.pl
@@ -15,9 +15,11 @@ scenarios(simulator => 1);
 # closely enough to pass the same test?
 # If not -- probably we should switch this to be vlt-only.
 
-compile();
+compile(verilator_flags2 => ["--trace"]);
 
 execute(check_finished => 1);
 
+vcd_identical("$Self->{obj_dir}/simx.vcd", "t/$Self->{name}.out");
+
 ok(1);
 1;