Mario1159/verilator
1
0
Fork 0
forked from github/verilator
Code Pull Requests Releases 1 Activity

Add --no-relative-cfuncs and related default optimization, bug1224.

Browse Source 
Signed-off-by: Wilson Snyder <wsnyder@wsnyder.org>
This commit is contained in:
John Coiner 2017-10-05 18:18:11 -04:00 committed by Wilson Snyder
parent 8281ee1520
commit ba270e09a4
8 changed files with 195 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Changes
View File

@ -11,6 +11,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
*** Add --x-initial option for specifying initial value assignment behavior.
*** Add --no-relative-cfuncs and related default optimization, bug1224. [John Coiner]
**** The internal test_verilated test directory is moved to be part of test_regress.
**** Fix over-aggressive inlining, bug1223. [John Coiner]

17
bin/verilator
View File

@ -328,6 +328,7 @@ descriptions in the next sections for more information.
--public Debugging; see docs
-pvalue+<name>=<value> Overwrite toplevel parameter
--relative-includes Resolve includes relative to current file
--no-relative-cfuncs Disallow 'this->' in generated functions
--report-unoptflat Extra diagnostics for UNOPTFLAT
--savable Enable model save-restore
--sc Create SystemC output
@ -880,6 +881,22 @@ change the ultimate model's performance, but may in some cases.
Backward compatible alias for "--pins-bv 33".
=item --no-relative-cfuncs
Disable 'this->' references in generated functions, and instead Verilator
will generate absolute references starting from 'vlTOPp->'. This prevents
V3Combine from merging functions from multiple instances of the same
module, so it can grow the instruction stream.
This is a work around for old compilers. Don't set this if your C++
compiler supports __restrict__ properly, as GCC 4.5.x and newer do. For
older compilers, test if this switch gives you better performance or not.
Compilers which don't honor __restrict__ will suspect that 'this->'
references and 'vlTOPp->' references may alias, and may write slow code
with extra loads and stores to handle the (imaginary) aliasing. Using only
'vlTOPp->' references allows these old compilers to produce tight code.
=item --no-skip-identical
Rarely needed. Disables skipping execution of Verilator if all source

135
src/V3Descope.cpp
View File

@ -53,7 +53,8 @@ private:
// STATE
AstNodeModule* m_modp; // Current module
AstScope* m_scopep; // Current scope
bool m_needThis; // Add thisp to function
bool m_modSingleton; // m_modp is only instanced once
bool m_needThis; // Make function non-static
FuncMmap m_modFuncs; // Name of public functions added
// METHODS
@ -63,41 +64,91 @@ private:
return level;
}
string descopedName(AstScope* scopep, bool& hierThisr, AstVar* varp=NULL) {
static bool modIsSingleton(AstNodeModule* modp) {
// True iff there's exactly one instance of this module in the design.
int instances = 0;
for (AstNode* stmtp = modp->stmtsp(); stmtp; stmtp=stmtp->nextp()) {
if (stmtp->castScope()) {
if (++instances > 1) { return false; }
}
}
return (instances == 1);
}
// Construct the best prefix to reference an object in 'scopep'
// from a CFunc in 'm_scopep'. Result may be relative
// ("this->[...]") or absolute ("vlTOPp->[...]").
//
// Using relative references allows V3Combine'ing
// code across multiple instances of the same module.
//
// Sets 'hierThisr' true if the object is local to this scope
// (and could be made into a function-local later in V3Localize),
// false if the object is in another scope.
string descopedName(const AstScope* scopep, bool& hierThisr,
const AstVar* varp=NULL) {
UASSERT(scopep, "Var/Func not scoped\n");
hierThisr = true;
hierThisr = (scopep == m_scopep);
// It's possible to disable relative references. This is a concession
// to older compilers (gcc < 4.5.x) that don't understand __restrict__
// well and emit extra ld/st to guard against pointer aliasing
// when this-> and vlTOPp-> are mixed in the same function.
//
// "vlTOPp" is declared "restrict" so better compilers understand
// that it won't alias with "this".
bool relativeRefOk = v3Global.opt.relativeCFuncs();
// 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
// become true OO routines.
//
// V3Combine wouldn't likely be able to combine top-level
// routines anyway, so there's no harm in keeping these static.
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.
// The risk that this prevents combining identical logic from differently-
// named but identical modules seems low.
if (m_modSingleton) {
relativeRefOk = false;
}
if (varp && varp->isFuncLocal()) {
hierThisr = true;
return ""; // Relative to function, not in this
} else if (scopep == m_scopep && m_modp->isTop()) {
//return ""; // Reference to scope we're in, no need to HIER-> it
return "vlTOPp->";
} else if (scopep == m_scopep && !m_modp->isTop()
&& 0) { // We no longer thisp-> as still get ambiguation problems
m_needThis = true;
return "thisp->"; // this-> but with restricted aliasing
} else if (scopep->aboveScopep() && scopep->aboveScopep()==m_scopep
&& 0 // DISABLED: GCC considers the pointers ambiguous, so goes ld/store crazy
) {
// Reference to scope of cell directly under this module, can just "cell->"
string name = scopep->name();
string::size_type pos;
if ((pos = name.rfind(".")) != string::npos) {
name.erase(0,pos+1);
}
hierThisr = false;
return name+"->";
} else {
// Reference to something else, use global variable
UINFO(8," Descope "<<scopep<<endl);
UINFO(8," to "<<scopep->name()<<endl);
UINFO(8," under "<<m_scopep->name()<<endl);
hierThisr = false;
if (!scopep->aboveScopep()) { // Top
return "vlTOPp->"; // == "vlSymsp->TOPp->", but GCC would suspect aliases
} else {
return scopep->nameVlSym()+".";
}
}
} else if (relativeRefOk && scopep == m_scopep) {
m_needThis = true;
return "this->";
} else if (relativeRefOk && scopep->aboveScopep()
&& scopep->aboveScopep()==m_scopep) {
// Reference to scope of cell directly under this module, can just "cell->"
string name = scopep->name();
string::size_type pos;
if ((pos = name.rfind(".")) != string::npos) {
name.erase(0,pos+1);
}
m_needThis = true;
return name+"->";
} else {
// Reference to something elsewhere, or relative refences
// are disabled. Use global variable
UINFO(8," Descope "<<scopep<<endl);
UINFO(8," to "<<scopep->name()<<endl);
UINFO(8," under "<<m_scopep->name()<<endl);
if (!scopep->aboveScopep()) { // Top
// We could also return "vlSymsp->TOPp->" here, but GCC would
// suspect aliases.
return "vlTOPp->";
} else {
return scopep->nameVlSym()+".";
}
}
}
void makePublicFuncWrappers() {
@ -182,6 +233,7 @@ private:
virtual void visit(AstNodeModule* nodep) {
m_modp = nodep;
m_modFuncs.clear();
m_modSingleton = modIsSingleton(m_modp);
nodep->iterateChildren(*this);
makePublicFuncWrappers();
m_modp = NULL;
@ -222,11 +274,7 @@ private:
nodep->iterateChildren(*this);
nodep->user1(true);
if (m_needThis) {
nodep->v3fatalSrc("old code");
// Really we should have more node types for backend optimization of this stuff
string text = v3Global.opt.modPrefix() + "_" + m_modp->name()
+"* thisp = &("+m_scopep->nameVlSym()+");\n";
nodep->addInitsp(new AstCStmt(nodep->fileline(), text));
nodep->isStatic(false);
}
// If it's under a scope, move it up to the top
if (m_scopep) {
@ -248,11 +296,12 @@ private:
}
public:
// CONSTRUCTORS
explicit DescopeVisitor(AstNetlist* nodep) {
m_modp = NULL;
m_scopep = NULL;
m_needThis = false;
nodep->accept(*this);
explicit DescopeVisitor(AstNetlist* nodep)
: m_modp(NULL),
m_scopep(NULL),
m_modSingleton(false),
m_needThis(false) {
nodep->accept(*this);
}
virtual ~DescopeVisitor() {}
};

2
src/V3Localize.cpp
View File

@ -84,7 +84,7 @@ private:
virtual void visit(AstVarRef* nodep) {
VarFlags flags (nodep->varp());
if (flags.m_done) {
nodep->hiername(""); // Remove thisp->
nodep->hiername(""); // Remove this->
nodep->hierThis(true);
}
}

6
src/V3Options.cpp
View File

@ -677,8 +677,9 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, char
else if ( onoff (sw, "-profile-cfuncs", flag/*ref*/) ) { m_profileCFuncs = flag; }
else if ( onoff (sw, "-public", flag/*ref*/) ) { m_public = flag; }
else if ( !strncmp(sw, "-pvalue+", strlen("-pvalue+"))) { addParameter(string(sw+strlen("-pvalue+")), false); }
else if ( onoff (sw, "-report-unoptflat", flag/*ref*/) ) { m_reportUnoptflat = flag; }
else if ( onoff (sw, "-relative-cfuncs", flag/*ref*/) ) { m_relativeCFuncs = flag; }
else if ( onoff (sw, "-relative-includes", flag/*ref*/) ) { m_relativeIncludes = flag; }
else if ( onoff (sw, "-report-unoptflat", flag/*ref*/) ) { m_reportUnoptflat = flag; }
else if ( onoff (sw, "-savable", flag/*ref*/) ) { m_savable = flag; }
else if ( !strcmp (sw, "-sc") ) { m_outFormatOk = true; m_systemC = true; }
else if ( onoff (sw, "-skip-identical", flag/*ref*/) ) { m_skipIdentical = flag; }
@ -1209,8 +1210,9 @@ V3Options::V3Options() {
m_preprocOnly = false;
m_preprocNoLine = false;
m_public = false;
m_reportUnoptflat = false;
m_relativeCFuncs = true;
m_relativeIncludes = false;
m_reportUnoptflat = false;
m_savable = false;
m_skipIdentical = true;
m_stats = false;

4
src/V3Options.h
View File

@ -88,8 +88,9 @@ class V3Options {
bool m_pinsUint8; // main switch: --pins-uint8
bool m_profileCFuncs;// main switch: --profile-cfuncs
bool m_public; // main switch: --public
bool m_reportUnoptflat; // main switch: --report-unoptflat
bool m_relativeCFuncs; // main switch: --relative-cfuncs
bool m_relativeIncludes; // main switch: --relative-includes
bool m_reportUnoptflat; // main switch: --report-unoptflat
bool m_savable; // main switch: --savable
bool m_systemC; // main switch: --sc: System C instead of simple C++
bool m_skipIdentical;// main switch: --skip-identical
@ -244,6 +245,7 @@ class V3Options {
bool lintOnly() const { return m_lintOnly; }
bool ignc() const { return m_ignc; }
bool inhibitSim() const { return m_inhibitSim; }
bool relativeCFuncs() const { return m_relativeCFuncs; }
bool reportUnoptflat() const { return m_reportUnoptflat; }
bool vpi() const { return m_vpi; }
bool xInitialEdge() const { return m_xInitialEdge; }

36
test_regress/t/t_inst_tree_inl0_pub1.pl
View File

@ -13,8 +13,42 @@ compile (
verilator_flags2 => ['+define+NOUSE_INLINE', '+define+USE_PUBLIC', '--stats'],
);
sub checkRelativeRefs {
my ($mod, $expect_relative) = @_;
my $found_relative = 0;
my $file = "$Self->{obj_dir}/V$Self->{name}_${mod}.cpp";
my $text = file_contents($file);
# Remove "this->__VlSymsp" which is noise
$text =~ s/this->__VlSymsp//g;
if ($text =~ m/this->/) {
$found_relative = 1;
}
if ($found_relative != $expect_relative) {
$Self->error("$file " .
($found_relative ? "has" : "does not have") .
" relative variable references.");
}
}
if ($Self->{vlt}) {
file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 16);
# We expect to combine sequent functions across multiple instances of
# l2, l3, l4, l5. If this number drops, please confirm this has not broken.
file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 59);
# Expect absolute refs in CFuncs for t (top module) and l1 (because it
# has only one instance)
checkRelativeRefs("t", 0);
checkRelativeRefs("l1", 0);
# Others should get relative references
checkRelativeRefs("l2", 1);
checkRelativeRefs("l3", 1);
checkRelativeRefs("l4", 1);
checkRelativeRefs("l5__P1", 1);
checkRelativeRefs("l5__P2", 1);
}
execute (

41
test_regress/t/t_inst_tree_inl0_pub1_norelcfuncs.pl Executable file
View File

@ -0,0 +1,41 @@
#!/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.
top_filename("t/t_inst_tree.v");
compile (
verilator_flags2 => ['+define+NOUSE_INLINE', '+define+USE_PUBLIC', '--stats', '--norelative-cfuncs'],
);
if ($Self->{vlt}) {
# Fewer optimizations than t_inst_tree_inl0_pub1 which allows
# relative CFuncs:
file_grep ($Self->{stats}, qr/Optimizations, Combined CFuncs\s+(\d+)/i, 16);
# Should not find any 'this->' except some 'this->__VlSymsp'
my @files = `ls $Self->{obj_dir}/*.cpp`;
foreach my $file (@files) {
chomp $file;
my $text = file_contents($file);
$text =~ s/this->__VlSymsp//g;
if ($text =~ m/this->/) {
$Self->error("$file has unexpected this-> refs when --norelative-cfuncs");
}
}
}
execute (
check_finished=>1,
expect=>
'\] (%m|.*t\.ps): Clocked
',
);
ok(1);
1;