Fix clock-gates with non-AND complex logic, bug220.

Changes

@@ -11,6 +11,8 @@ indicates the contributor was also the author of the fix; Thanks!
 
 **** Skip SystemC tests if not installed.  [Iztok Jeras]
 
+**** Fix clock-gates with non-AND complex logic, bug220. [Ashutosh Das]
+
 **** Fix flushing VCD buffers on $stop.  [Ashutosh Das]
 
 **** Fix Mac OS-X compile issues, bug217.  [Joshua Wise, Trevor Williams]

bin/verilator

@@ -1629,7 +1629,8 @@ Used after a signal declaration to indicate the signal is used to gate a
 clock, and the user takes responsibility for insuring there are no races
 related to it.  (Typically by adding a latch, and running static timing
 analysis.) This will cause the clock gate to be ignored in the scheduling
-algorithm, improving performance.
+algorithm, improving performance.  It's also a good idea to enable the
+IMPERFECTSCH warning, to insure all clock enables are properly recognized.
 
 =item /*verilator coverage_block_off*/
 

src/V3Order.cpp

@@ -941,28 +941,51 @@ void OrderVisitor::processBrokeLoop() {
 // Clock propagation
 
 void OrderVisitor::processInputs() {
-    m_graph.userClearVertices();  // Vertex::user()   // true if added as begin/end
+    m_graph.userClearVertices();  // Vertex::user()   // true if processed
+    // Start at input vertex, process from input-to-output order
+    m_inputsVxp->isFromInput(true);  // By definition
     processInputsIterate(m_inputsVxp);
 }
 
 void OrderVisitor::processInputsIterate(OrderEitherVertex* vertexp) {
     // Propagate PrimaryIn through simple assignments
     if (vertexp->user()) return;  // Already processed
-    //UINFO(9," InIt "<<vertexp<<endl);
-    vertexp->user(true);
-    if (OrderVarStdVertex* vvertexp = dynamic_cast<OrderVarStdVertex*>(vertexp)) {
-	vvertexp->isFromInput(true);
-    }
-    for (V3GraphEdge* edgep = vertexp->outBeginp(); edgep; edgep=edgep->outNextp()) {
-	OrderEitherVertex* toVertexp = (OrderEitherVertex*)edgep->top();
-	if (OrderVarStdVertex* vvertexp = dynamic_cast<OrderVarStdVertex*>(toVertexp)) {
-	    processInputsIterate(vvertexp);
+    if (0 && debug()>=9) {
+	UINFO(9," InIt "<<vertexp<<endl);
+	if (OrderLogicVertex* vvertexp = dynamic_cast<OrderLogicVertex*>(vertexp)) {
+	    vvertexp->nodep()->dumpTree(cout,"-            TT: ");
 	}
-	if (OrderLogicVertex* vvertexp = dynamic_cast<OrderLogicVertex*>(toVertexp)) {
-	    if (AstNodeAssign* nodep = vvertexp->nodep()->castNodeAssign()) {
-		if (nodep->lhsp()->castVarRef()
-		    && nodep->rhsp()->castVarRef()) {
-		    UINFO(9,"   Input reassignment: "<<vvertexp<<endl);
+    }
+    vertexp->user(true);  // Processing
+    // First handle all inputs to this vertex, in most cases they'll be already processed earlier
+    // Also, determine if this vertex is an input
+    int inonly = 1;  // 0=no, 1=maybe, 2=yes until a no
+    for (V3GraphEdge* edgep = vertexp->inBeginp(); edgep; edgep=edgep->inNextp()) {
+	OrderEitherVertex* frVertexp = (OrderEitherVertex*)edgep->fromp();
+	processInputsIterate(frVertexp);
+	if (frVertexp->isFromInput()) {
+	    if (inonly==1) inonly = 2;
+	} else if (dynamic_cast<OrderVarPostVertex*>(frVertexp)) {
+	    // Ignore post assignments, just for ordering
+	} else {
+	    //UINFO(9,"    InItStopDueTo "<<frVertexp<<endl);
+	    inonly = 0;
+	    break;
+	}
+    }
+    if (inonly == 2) {  // Set it.  Note may have already been set earlier, too
+	UINFO(9,"   Input reassignment: "<<vertexp<<endl);
+	vertexp->isFromInput(true);
+    }
+    // If we're still an input, process all targets of this vertex
+    if (vertexp->isFromInput()) {
+	for (V3GraphEdge* edgep = vertexp->outBeginp(); edgep; edgep=edgep->outNextp()) {
+	    OrderEitherVertex* toVertexp = (OrderEitherVertex*)edgep->top();
+	    if (OrderVarStdVertex* vvertexp = dynamic_cast<OrderVarStdVertex*>(toVertexp)) {
+		processInputsIterate(vvertexp);
+	    }
+	    if (OrderLogicVertex* vvertexp = dynamic_cast<OrderLogicVertex*>(toVertexp)) {
+		if (vvertexp->nodep()->castNodeAssign()) {
 		    processInputsIterate(vvertexp);
 		}
 	    }

src/V3OrderGraph.h

@@ -132,10 +132,11 @@ class OrderEitherVertex : public V3GraphVertex {
     AstScope*	m_scopep;	// Scope the vertex is in
     AstSenTree*	m_domainp;	// Clock domain (NULL = to be computed as we iterate)
     OrderLoopId	m_inLoop;	// Loop number vertex is in
+    bool	m_isFromInput;	// From input, or derrived therefrom (conservatively false)
 public:
     OrderEitherVertex(V3Graph* graphp, AstScope* scopep, AstSenTree* domainp)
 	: V3GraphVertex(graphp), m_scopep(scopep), m_domainp(domainp)
-	, m_inLoop(LOOPID_UNKNOWN) {
+	, m_inLoop(LOOPID_UNKNOWN), m_isFromInput(false) {
     }
     virtual ~OrderEitherVertex() {}
     // Methods
@@ -148,12 +149,16 @@ public:
     AstSenTree* domainp() const { return m_domainp; }
     OrderLoopId	inLoop() const { return m_inLoop; }
     void inLoop(OrderLoopId inloop) { m_inLoop = inloop; }
+    void isFromInput(bool flag) { m_isFromInput=flag; }
+    bool isFromInput() const { return m_isFromInput; }
 };
 
 class OrderInputsVertex : public OrderEitherVertex {
 public:
     OrderInputsVertex(V3Graph* graphp, AstSenTree* domainp)
-	: OrderEitherVertex(graphp, NULL, domainp) {}
+	: OrderEitherVertex(graphp, NULL, domainp) {
+	isFromInput(true);  // By definition
+    }
     virtual ~OrderInputsVertex() {}
     virtual OrderVEdgeType type() const { return OrderVEdgeType::VERTEX_INPUTS; }
     virtual string name() const { return "*INPUTS*"; }
@@ -195,11 +200,10 @@ class OrderVarVertex : public OrderEitherVertex {
     AstVarScope* m_varScp;
     OrderVarVertex*	m_pilNewVertexp;	// for processInsLoopNewVar
     bool	 m_isClock;	// Used as clock
-    bool	 m_isFromInput;	// From input, or derrived therefrom (conservatively false)
 public:
     OrderVarVertex(V3Graph* graphp, AstScope* scopep, AstVarScope* varScp)
 	: OrderEitherVertex(graphp, scopep, NULL), m_varScp(varScp)
-	, m_pilNewVertexp(NULL), m_isClock(false), m_isFromInput(false)
+	, m_pilNewVertexp(NULL), m_isClock(false)
 	{}
     virtual ~OrderVarVertex() {}
     virtual OrderVarVertex* clone (V3Graph* graphp) const = 0;
@@ -208,8 +212,6 @@ public:
     AstVarScope* varScp() const { return m_varScp; }
     void isClock(bool flag) { m_isClock=flag; }
     bool isClock() const { return m_isClock; }
-    void isFromInput(bool flag) { m_isFromInput=flag; }
-    bool isFromInput() const { return m_isFromInput; }
     OrderVarVertex* pilNewVertexp() const { return m_pilNewVertexp; }
     void pilNewVertexp (OrderVarVertex* vertexp) { m_pilNewVertexp = vertexp; }
 };

test_regress/driver.pl

@@ -183,7 +183,7 @@ sub parameter {
     elsif ($param =~ /\.pl/) {
 	push @opt_tests, $param;
     }
-    elsif ($param =~ /^--debugi/) {
+    elsif ($param =~ /^-?-debugi/) {
 	push @Opt_Driver_Verilator_Flags, $param;
 	$_Parameter_Next_Level = $param;
     }

test_regress/t/t_clk_latchgate.pl

@@ -0,0 +1,18 @@
+#!/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.
+
+compile (
+	 );
+
+execute (
+	 check_finished=>1,
+     );
+
+ok(1);
+1;

test_regress/t/t_clk_latchgate.v

@@ -0,0 +1,178 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2010 by Wilson Snyder.
+//
+//    --------------------------------------------------------
+//    Bug Description:
+//
+//    Issue:  The gated clock gclk_vld[0] toggles but dvld[0]
+//    input to the flop does not propagate to the output
+//    signal entry_vld[0] correctly. The value that propagates
+//    is the new value of dvld[0] not the one just before the
+//    posedge of gclk_vld[0].
+//    --------------------------------------------------------
+
+// Define to see the bug with test failing with gated clock 'gclk_vld'
+// Comment out the define to see the test passing with ungated clock 'clk'
+`define GATED_CLK_TESTCASE 1
+
+// A side effect of the problem is this warning, disabled by default
+//verilator lint_on IMPERFECTSCH
+
+// Test Bench
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+   integer 	cyc=0;
+   reg [63:0] 	crc;
+
+   // Take CRC data and apply to testblock inputs
+   wire [7:0]  dvld = crc[7:0];
+   wire [7:0]  ff_en_e1 = crc[15:8];
+
+   /*AUTOWIRE*/
+   // Beginning of automatic wires (for undeclared instantiated-module outputs)
+   wire [7:0]		entry_vld;		// From test of Test.v
+   wire [7:0]		ff_en_vld;		// From test of Test.v
+   // End of automatics
+
+   Test test (/*AUTOINST*/
+	      // Outputs
+	      .ff_en_vld		(ff_en_vld[7:0]),
+	      .entry_vld		(entry_vld[7:0]),
+	      // Inputs
+	      .clk			(clk),
+	      .dvld			(dvld[7:0]),
+	      .ff_en_e1			(ff_en_e1[7:0]));
+
+   reg err_code;
+   reg ffq_clk_active;
+   reg [7:0] prv_dvld;
+
+   initial begin
+     err_code = 0;
+     ffq_clk_active = 0;
+   end
+   always @ (posedge clk) begin
+     prv_dvld = test.dvld;
+   end
+   always @ (negedge test.ff_entry_dvld_0.clk) begin
+     ffq_clk_active = 1;
+     if (test.entry_vld[0] !== prv_dvld[0]) err_code = 1;
+   end
+
+   // Test loop
+   always @ (posedge clk) begin
+`ifdef TEST_VERBOSE
+      $write("[%0t] cyc==%0d crc=%x ",$time, cyc, crc);
+      $display(" en=%b fen=%b d=%b ev=%b",
+	       test.flop_en_vld[0],  test.ff_en_vld[0],
+	       test.dvld[0],  test.entry_vld[0]);
+`endif
+      cyc <= cyc + 1;
+      crc <= {crc[62:0], crc[63]^crc[2]^crc[0]};
+      if (cyc<3) begin
+	 crc <= 64'h5aef0c8d_d70a4497;
+      end
+      else if (cyc==99) begin
+	 $write("[%0t] cyc==%0d crc=%x\n",$time, cyc, crc);
+         if (ffq_clk_active == 0) begin
+           $display ("----");
+           $display ("%%Error: TESTCASE FAILED with no Clock arriving at FFQs");
+           $display ("----");
+           $stop;
+         end
+         else if (err_code) begin
+           $display ("----");
+           $display ("%%Error: TESTCASE FAILED with invalid propagation of 'd' to 'q' of FFQs");
+           $display ("----");
+           $stop;
+         end
+         else begin
+	   $write("*-* All Finished *-*\n");
+	   $finish;
+        end
+      end
+   end
+
+endmodule
+
+module llq (clk, d, q);
+   parameter WIDTH = 32;
+   input clk;
+   input [WIDTH-1:0] d;
+   output [WIDTH-1:0] q;
+
+   reg [WIDTH-1:0]    qr;
+
+   /* verilator lint_off COMBDLY */
+
+   always @(clk or d)
+     if (clk == 1'b0)
+       qr <= d;
+
+   /* verilator lint_on COMBDLY */
+
+   assign q = qr;
+endmodule
+
+module ffq (clk, d, q);
+   parameter WIDTH = 32;
+   input clk;
+   input [WIDTH-1:0] d;
+   output [WIDTH-1:0] q;
+
+   reg [WIDTH-1:0]    qr;
+
+   always @(posedge clk)
+     qr <= d;
+
+   assign q = qr;
+endmodule
+
+// DUT module
+module Test (/*AUTOARG*/
+   // Outputs
+   ff_en_vld, entry_vld,
+   // Inputs
+   clk, dvld, ff_en_e1
+   );
+   input clk;
+
+   input [7:0] dvld;
+   input [7:0] ff_en_e1;
+
+   output [7:0] ff_en_vld;
+   output wire [7:0] entry_vld;
+
+   wire [7:0] gclk_vld;
+   wire [7:0] ff_en_vld /*verilator clock_enable*/;
+   reg [7:0] flop_en_vld;
+
+   always @(posedge clk) flop_en_vld <= ff_en_e1;
+
+   // clock gating
+`ifdef GATED_CLK_TESTCASE
+   assign gclk_vld = {8{clk}} & ff_en_vld;
+`else
+   assign gclk_vld = {8{clk}};
+`endif
+
+   // latch for avoiding glitch on the clock gating control
+   llq  #(8)  dp_ff_en_vld (.clk(clk), .d(flop_en_vld), .q(ff_en_vld));
+
+   // flops that use the gated clock signal
+   ffq  #(1)  ff_entry_dvld_0 (.clk(gclk_vld[0]), .d(dvld[0]), .q(entry_vld[0]));
+   ffq  #(1)  ff_entry_dvld_1 (.clk(gclk_vld[1]), .d(dvld[1]), .q(entry_vld[1]));
+   ffq  #(1)  ff_entry_dvld_2 (.clk(gclk_vld[2]), .d(dvld[2]), .q(entry_vld[2]));
+   ffq  #(1)  ff_entry_dvld_3 (.clk(gclk_vld[3]), .d(dvld[3]), .q(entry_vld[3]));
+   ffq  #(1)  ff_entry_dvld_4 (.clk(gclk_vld[4]), .d(dvld[4]), .q(entry_vld[4]));
+   ffq  #(1)  ff_entry_dvld_5 (.clk(gclk_vld[5]), .d(dvld[5]), .q(entry_vld[5]));
+   ffq  #(1)  ff_entry_dvld_6 (.clk(gclk_vld[6]), .d(dvld[6]), .q(entry_vld[6]));
+   ffq  #(1)  ff_entry_dvld_7 (.clk(gclk_vld[7]), .d(dvld[7]), .q(entry_vld[7]));
+
+endmodule