Fix mis-optimizing public DPI functions, bug963.

Changes

@@ -5,6 +5,8 @@ indicates the contributor was also the author of the fix; Thanks!
 
 * Verilator 3.877 devel
 
+****  Fix mis-optimizing public DPI functions, bug963. [Wei Song]
+
 
 * Verilator 3.876 2015-08-12
 

src/V3Ast.cpp

@@ -863,6 +863,16 @@ void AstNode::cloneRelinkTree() {
 //======================================================================
 // Comparison
 
+bool AstNode::gateTreeIter() {
+    // Return true if the two trees are identical
+    if (this==NULL) return true;
+    if (!isGateOptimizable()) return false;
+    return (this->op1p()->gateTreeIter()
+	    && this->op2p()->gateTreeIter()
+	    && this->op3p()->gateTreeIter()
+	    && this->op4p()->gateTreeIter());
+}
+
 bool AstNode::sameTreeIter(AstNode* node2p, bool ignNext, bool gateOnly) {
     // Return true if the two trees are identical
     if (this==NULL && node2p==NULL) return true;

src/V3Ast.h

@@ -974,12 +974,15 @@ class AstNode {
 
     void	init();	// initialize value of AstNode
     void	iterateListBackwards(AstNVisitor& v, AstNUser* vup=NULL);
+private:
     AstNode*	cloneTreeIter();
     AstNode*	cloneTreeIterList();
     void	checkTreeIter(AstNode* backp);
     void	checkTreeIterList(AstNode* backp);
+    bool	gateTreeIter();
     bool	sameTreeIter(AstNode* node2p, bool ignNext, bool gateOnly);
     void	deleteTreeIter();
+public:
     void	deleteNode();
     static void	relinkOneLink(AstNode*& pointpr, AstNode* newp);
     // cppcheck-suppress functionConst
@@ -1207,6 +1210,7 @@ public:
 
     // METHODS - Iterate on a tree
     AstNode*	cloneTree(bool cloneNextLink);
+    bool	gateTree() { return gateTreeIter(); }  // Is tree gateOptimizable?
     bool	sameTree(AstNode* node2p);	// Does tree of this == node2p?
     bool	sameGateTree(AstNode* node2p);	// Does tree of this == node2p?, not allowing non-isGateOptimizable
     void	deleteTree();	// Always deletes the next link
@@ -1790,6 +1794,7 @@ public:
     virtual void dump(ostream& str=cout);
     virtual string name()	const { return m_name; }		// * = Var name
     virtual bool maybePointedTo() const { return true; }
+    virtual bool isGateOptimizable() const { return !((m_dpiExport || m_dpiImport) && !m_pure); }
     // {AstFunc only} op1 = Range output variable
     virtual void name(const string& name) 	{ m_name = name; }
     string cname() const { return m_cname; }
@@ -1848,6 +1853,7 @@ public:
     }}
     virtual void dump(ostream& str=cout);
     virtual string name()	const { return m_name; }		// * = Var name
+    virtual bool isGateOptimizable() const { return m_taskp && m_taskp->isGateOptimizable(); }
     string	dotted()	const { return m_dotted; }		// * = Scope name or ""
     string	prettyDotted() const { return prettyName(dotted()); }
     string	inlinedDots() const { return m_inlinedDots; }

src/V3Const.cpp

@@ -450,6 +450,9 @@ private:
 	if (!ifvarp || !elsevarp) return false;
 	if (ifvarp->isWide()) return false;  // Would need temporaries, so not worth it
 	if (!ifvarp->sameGateTree(elsevarp)) return false;
+	UINFO(1,"HERE "<<nodep<<endl);
+	if (!ifp->rhsp()->gateTree()) return false;
+	if (!elsep->rhsp()->gateTree()) return false;
 	return true;
     }
     bool operandIfIf(AstNodeIf* nodep) {

test_regress/t/t_dpi_shortcircuit.v

@@ -19,15 +19,14 @@ module t (/*AUTOARG*/);
 
    // Note these are NOT pure.
    import "DPI-C" function int dpii_clear ();
-   import "DPI-C" function int dpii_count (input int i);
-   import "DPI-C" function bit dpii_inc0  (input int i);
-   import "DPI-C" function bit dpii_inc1  (input int i);
-   import "DPI-C" function bit dpii_incx  (input int i, input bit value);
+   import "DPI-C" function int dpii_count (input int ctr);
+   import "DPI-C" function bit dpii_inc0  (input int ctr);
+   import "DPI-C" function bit dpii_inc1  (input int ctr);
+   import "DPI-C" function bit dpii_incx  (input int ctr, input bit value);
 
-   // verilator lint_off UNUSED
    integer i;
+   integer j;
    bit 	   b;
-   // verilator lint_on UNUSED
    integer errors;
 
    task check1(integer line, bit got, bit ex);

test_regress/t/t_dpi_shortcircuit2.pl

@@ -0,0 +1,21 @@
+#!/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 (
+	 # Amazingly VCS, NC and Verilator all just accept the C file here!
+	 v_flags2 => ["t/t_dpi_shortcircuit_c.cpp"],
+	 verilator_flags2 => ["-Wno-DECLFILENAME"],
+	 );
+
+execute (
+	 check_finished=>1,
+     );
+
+ok(1);
+1;

test_regress/t/t_dpi_shortcircuit2.v

@@ -0,0 +1,66 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// Copyright 2009 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.
+
+`ifdef VCS
+ `define NO_SHORTREAL
+`endif
+`ifdef NC
+ `define NO_SHORTREAL
+`endif
+`ifdef VERILATOR  // Unsupported
+ `define NO_SHORTREAL
+`endif
+
+module t (/*AUTOARG*/);
+
+   // Note these are NOT pure.
+   import "DPI-C" function int dpii_clear ();
+   import "DPI-C" function int dpii_count (input int ctr);
+   import "DPI-C" function bit dpii_inc0  (input int ctr);
+   import "DPI-C" function bit dpii_inc1  (input int ctr);
+   import "DPI-C" function bit dpii_incx  (input int ctr, input bit value);
+
+   integer i;
+   integer j;
+   integer k;
+   bit 	   b;
+   integer errors;
+
+   task check1(integer line, bit got, bit ex);
+      if (got != ex) begin
+	 $display("%%Error: Line %0d: Bad result, got=%0d expect=%0d",line,got,ex);
+	 errors++;
+      end
+   endtask
+   task check(integer line, int got, int ex);
+      if (got != ex) begin
+	 $display("%%Error: Line %0d: Bad result, got=%0d expect=%0d",line,got,ex);
+	 errors++;
+      end
+   endtask
+
+   // Test loop
+   initial begin
+      // bug963
+      dpii_clear();
+      j = 0;
+      for (i=0; i<64; i++) begin
+	 if (i[0])
+	   j = 0;
+	 else 
+	   j = {31'b0, dpii_inc1(0)};
+	 k = k + j;
+      end
+      $write("%x\n",k);
+      check (`__LINE__, dpii_count(0), 32);
+
+      if (|errors) $stop;
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+
+endmodule

test_regress/t/t_dpi_shortcircuit_c.cpp

@@ -20,7 +20,13 @@
 //======================================================================
 
 #if defined(VERILATOR)
-# include "Vt_dpi_shortcircuit__Dpi.h"
+# if defined(T_DPI_SHORTCIRCUIT)
+#  include "Vt_dpi_shortcircuit__Dpi.h"
+# elif defined(T_DPI_SHORTCIRCUIT2)
+#  include "Vt_dpi_shortcircuit2__Dpi.h"
+# else
+#  error "Unknown test"
+# endif
 #elif defined(VCS)
 # include "../vc_hdrs.h"
 #elif defined(CADENCE)
@@ -55,5 +61,5 @@ unsigned char dpii_incx (int idx, unsigned char value) {
     if (idx >= 0 && idx<COUNTERS) global_count[idx]++;
     return value;
 }
-unsigned char dpii_inc0 (int idx) { return dpii_incx(idx,false); }
-unsigned char dpii_inc1 (int idx) { return dpii_incx(idx,true); }
+unsigned char dpii_inc0 (int idx) { return dpii_incx(idx,0); }
+unsigned char dpii_inc1 (int idx) { return dpii_incx(idx,1); }