diff --git a/Changes b/Changes index 143508bb8..c59d2a971 100644 --- a/Changes +++ b/Changes @@ -23,6 +23,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix internal error on xrefs into unrolled functions, bug1387. [Al Grant] +**** Fix DPI export void compiler error, bug1391. [Stan Sokorac] + * Verilator 4.008 2018-12-01 diff --git a/src/V3Ast.h b/src/V3Ast.h index 36664bda4..935c76a01 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -510,6 +510,36 @@ public: //###################################################################### +/// Boolean or unknown +class VBoolOrUnknown { +public: + enum en { + BU_FALSE=0, + BU_TRUE=1, + BU_UNKNOWN=2, + _ENUM_END + }; + enum en m_e; + // CONSTRUCTOR - note defaults to *UNKNOWN* + inline VBoolOrUnknown() : m_e(BU_UNKNOWN) {} + // cppcheck-suppress noExplicitConstructor + inline VBoolOrUnknown(en _e) : m_e(_e) {} + explicit inline VBoolOrUnknown(int _e) : m_e(static_cast(_e)) {} + const char* ascii() const { + static const char* const names[] = { + "FALSE","TRUE","UNK"}; + return names[m_e]; } + bool trueU() const { return m_e == BU_TRUE || m_e == BU_UNKNOWN; } + bool falseU() const { return m_e == BU_FALSE || m_e == BU_UNKNOWN; } + bool unknown() const { return m_e == BU_UNKNOWN; } + }; + inline bool operator== (VBoolOrUnknown lhs, VBoolOrUnknown rhs) { return (lhs.m_e == rhs.m_e); } + inline bool operator== (VBoolOrUnknown lhs, VBoolOrUnknown::en rhs) { return (lhs.m_e == rhs); } + inline bool operator== (VBoolOrUnknown::en lhs, VBoolOrUnknown rhs) { return (lhs == rhs.m_e); } + inline std::ostream& operator<<(std::ostream& os, const VBoolOrUnknown& rhs) { return os<AstNode::dump(str); if (slow()) str<<" [SLOW]"; if (pure()) str<<" [PURE]"; - if (isStatic()) str<<" [STATIC]"; + if (isStatic().unknown()) str<<" [STATICU]"; + else if (isStatic().trueU()) str<<" [STATIC]"; if (dpiImport()) str<<" [DPII]"; if (dpiExport()) str<<" [DPIX]"; if (dpiExportWrapper()) str<<" [DPIXWR]"; diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 326a5fc4e..72ad1096c 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -5528,15 +5528,15 @@ private: string m_cname; // C name, for dpiExports string m_rtnType; // void, bool, or other return type string m_argTypes; - string m_ifdef; // #ifdef symbol around this function - bool m_dontCombine:1; // V3Combine shouldn't compare this func tree, it's special + string m_ifdef; // #ifdef symbol around this function + VBoolOrUnknown m_isStatic; // Function is declared static (no this) + bool m_dontCombine:1; // V3Combine shouldn't compare this func tree, it's special bool m_skipDecl:1; // Don't declare it bool m_declPrivate:1; // Declare it private bool m_formCallTree:1; // Make a global function to call entire tree of functions bool m_slow:1; // Slow routine, called once or just at init time bool m_funcPublic:1; // From user public task/function bool m_isInline:1; // Inline function - bool m_isStatic:1; // Function is declared static (no this) bool m_symProlog:1; // Setup symbol table for later instructions bool m_entryPoint:1; // User may call into this top level function bool m_pure:1; // Pure function @@ -5547,8 +5547,9 @@ private: public: AstCFunc(FileLine* fl, const string& name, AstScope* scopep, const string& rtnType="") : AstNode(fl) { - m_funcType = AstCFuncType::FT_NORMAL; - m_scopep = scopep; + m_funcType = AstCFuncType::FT_NORMAL; + m_isStatic = VBoolOrUnknown::BU_UNKNOWN; // Unknown until see where thisp needed + m_scopep = scopep; m_name = name; m_rtnType = rtnType; m_dontCombine = false; @@ -5558,7 +5559,6 @@ public: m_slow = false; m_funcPublic = false; m_isInline = false; - m_isStatic = true; // Note defaults to static, later we see where thisp is needed m_symProlog = false; m_entryPoint = false; m_pure = false; @@ -5582,7 +5582,10 @@ public: || name() == asamep->name())); } // virtual void name(const string& name) { m_name = name; } - virtual int instrCount() const { return dpiImport() ? instrCountDpi() : 0; } + virtual int instrCount() const { return dpiImport() ? instrCountDpi() : 0; } + VBoolOrUnknown isStatic() const { return m_isStatic; } + void isStatic(bool flag) { m_isStatic = flag ? VBoolOrUnknown::BU_TRUE : VBoolOrUnknown::BU_FALSE; } + void isStatic(VBoolOrUnknown flag) { m_isStatic = flag; } void cname(const string& name) { m_cname = name; } string cname() const { return m_cname; } AstScope* scopep() const { return m_scopep; } @@ -5609,8 +5612,6 @@ public: AstCFuncType funcType() const { return m_funcType; } bool isInline() const { return m_isInline; } void isInline(bool flag) { m_isInline = flag; } - bool isStatic() const { return m_isStatic; } - void isStatic(bool flag) { m_isStatic = flag; } bool symProlog() const { return m_symProlog; } void symProlog(bool flag) { m_symProlog = flag; } bool entryPoint() const { return m_entryPoint; } diff --git a/src/V3Depth.cpp b/src/V3Depth.cpp index 943312c48..34ce9df63 100644 --- a/src/V3Depth.cpp +++ b/src/V3Depth.cpp @@ -130,10 +130,10 @@ private: //-------------------- // Marking of non-static functions (because they might need "this") - // (Here just to avoid another iteration) + // (Here instead of new vistor after V3Descope just to avoid another visitor) void needNonStaticFunc(AstNode* nodep) { - if (!m_funcp) nodep->v3fatalSrc("Non-static accessor not under a function"); - if (m_funcp->isStatic()) { + if (!m_funcp) nodep->v3fatalSrc("Non-static accessor not under a function"); + if (m_funcp->isStatic().trueU()) { UINFO(5,"Mark non-public due to "<isStatic(false); } diff --git a/src/V3Descope.cpp b/src/V3Descope.cpp index f120c95ff..48b977afc 100644 --- a/src/V3Descope.cpp +++ b/src/V3Descope.cpp @@ -52,8 +52,9 @@ private: // STATE AstNodeModule* m_modp; // Current module AstScope* m_scopep; // Current scope - bool m_modSingleton; // m_modp is only instanced once - bool m_needThis; // Make function non-static + bool m_modSingleton; // m_modp is only instanced once + bool m_allowThis; // Allow function non-static + bool m_needThis; // Make function non-static FuncMmap m_modFuncs; // Name of public functions added // METHODS @@ -93,7 +94,11 @@ private: // "vlTOPp" is declared "restrict" so better compilers understand // that it won't alias with "this". bool relativeRefOk = v3Global.opt.relativeCFuncs(); + // + // Static functions can't use this + if (!m_allowThis) relativeRefOk = false; + // // Use absolute refs in top-scoped routines, keep them static. // The DPI callback registration depends on representing top-level // static routines as plain function pointers. That breaks if those @@ -104,7 +109,7 @@ private: if (m_modp->isTop()) { relativeRefOk = false; } - + // // Use absolute refs if this scope is the only instance of the module. // Saves a bit of overhead on passing the 'this' pointer, and there's no // need to be nice to V3Combine when we have only a single instance. @@ -266,9 +271,10 @@ private: virtual void visit(AstCFunc* nodep) { if (!nodep->user1()) { m_needThis = false; + m_allowThis = nodep->isStatic().falseU(); // Non-static or unknown if static iterateChildren(nodep); - nodep->user1(true); - if (m_needThis) { + nodep->user1(true); + if (m_needThis) { nodep->isStatic(false); } // If it's under a scope, move it up to the top @@ -295,6 +301,7 @@ public: : m_modp(NULL), m_scopep(NULL), m_modSingleton(false), + m_allowThis(false), m_needThis(false) { iterate(nodep); } diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index b83552f81..e0d5b06d8 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -2325,9 +2325,9 @@ void EmitCImp::emitIntFuncDecls(AstNodeModule* modp) { const AstCFunc* funcp = *it; if (!funcp->dpiImport()) { // DPI is prototyped in __Dpi.h ofp()->putsPrivate(funcp->declPrivate()); - if (funcp->ifdef()!="") puts("#ifdef "+funcp->ifdef()+"\n"); - if (funcp->isStatic()) puts("static "); - puts(funcp->rtnTypeVoid()); puts(" "); + if (funcp->ifdef()!="") puts("#ifdef "+funcp->ifdef()+"\n"); + if (funcp->isStatic().trueU()) puts("static "); + puts(funcp->rtnTypeVoid()); puts(" "); puts(funcp->name()); puts("("+cFuncArgs(funcp)+");\n"); if (funcp->ifdef()!="") puts("#endif // "+funcp->ifdef()+"\n"); } diff --git a/test_regress/t/t_dpi_export_c.cpp b/test_regress/t/t_dpi_export_c.cpp index b7a8bf379..6ec0b3ee7 100644 --- a/test_regress/t/t_dpi_export_c.cpp +++ b/test_regress/t/t_dpi_export_c.cpp @@ -26,7 +26,11 @@ //====================================================================== #if defined(VERILATOR) -# include "Vt_dpi_export__Dpi.h" +# ifdef T_DPI_EXPORT_NOOPT +# include "Vt_dpi_export_noopt__Dpi.h" +# else +# include "Vt_dpi_export__Dpi.h" +# endif #elif defined(VCS) # include "../vc_hdrs.h" #elif defined(CADENCE) @@ -93,10 +97,11 @@ static int check_sub(const char* name, int i) { svScope sout = svSetScope(scope); CHECK_RESULT(svScope, sout, prev); CHECK_RESULT(svScope, svGetScope(), scope); +#ifndef T_DPI_EXPORT_NOOPT int out = dpix_sub_inst(100*i); CHECK_RESULT(int, out, 100*i + i); - - return 0; // OK +#endif + return 0; // OK } // Called from our Verilog code to run the tests diff --git a/test_regress/t/t_dpi_export_noopt.pl b/test_regress/t/t_dpi_export_noopt.pl new file mode 100755 index 000000000..374aaf77b --- /dev/null +++ b/test_regress/t/t_dpi_export_noopt.pl @@ -0,0 +1,28 @@ +#!/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. + +# irun -sv top.v t_dpi_export.v -cpost t_dpi_export_c.c -end + +scenarios(simulator => 1); + +top_filename("t/t_dpi_export.v"); + +compile( + # Amazingly VCS, NC and Verilator all just accept the C file here! + v_flags2 => ["t/t_dpi_export_c.cpp"], + verilator_flags2 => ["-Wall -Wno-DECLFILENAME -no-l2name", + $Self->{vlt_all} ? "-O0" : ""], + ); + +execute( + check_finished => 1, + ); + +ok(1); +1;