diff --git a/Changes b/Changes index c5203dd44..179e4abeb 100644 --- a/Changes +++ b/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 diff --git a/src/V3Ast.cpp b/src/V3Ast.cpp index 43da4b077..8b577113a 100644 --- a/src/V3Ast.cpp +++ b/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; diff --git a/src/V3Ast.h b/src/V3Ast.h index 2560fd026..6e8c446c0 100644 --- a/src/V3Ast.h +++ b/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; } diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 2f2ba5341..a761be478 100644 --- a/src/V3Const.cpp +++ b/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 "<rhsp()->gateTree()) return false; + if (!elsep->rhsp()->gateTree()) return false; return true; } bool operandIfIf(AstNodeIf* nodep) { diff --git a/test_regress/t/t_dpi_shortcircuit.v b/test_regress/t/t_dpi_shortcircuit.v index 4bd1dc22a..c2ff2ac74 100644 --- a/test_regress/t/t_dpi_shortcircuit.v +++ b/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); diff --git a/test_regress/t/t_dpi_shortcircuit2.pl b/test_regress/t/t_dpi_shortcircuit2.pl new file mode 100755 index 000000000..216c1b691 --- /dev/null +++ b/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; diff --git a/test_regress/t/t_dpi_shortcircuit2.v b/test_regress/t/t_dpi_shortcircuit2.v new file mode 100644 index 000000000..602bdf9c2 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_dpi_shortcircuit_c.cpp b/test_regress/t/t_dpi_shortcircuit_c.cpp index b5024e5f4..25a31b8ab 100644 --- a/test_regress/t/t_dpi_shortcircuit_c.cpp +++ b/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