From cdeb6e792f4beb1faa96523b6f37f69d23b5e4fd Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sun, 25 Jul 2021 16:10:55 +0100 Subject: [PATCH] Add --instr-count-dpi option, change default to 200 This replaces the former static AstNode::INSTR_COUNT_DPI, and makes it user adjustable to fit the design. Fixes #3068. --- Changes | 4 +++- bin/verilator | 1 + docs/guide/exe_verilator.rst | 10 ++++++++++ docs/guide/verilating.rst | 6 ++++++ src/V3Ast.h | 1 - src/V3AstNodes.h | 4 +++- src/V3Options.cpp | 4 ++++ src/V3Options.h | 2 ++ test_regress/t/t_flag_instr_count_dpi_bad.pl | 20 ++++++++++++++++++++ 9 files changed, 49 insertions(+), 3 deletions(-) create mode 100755 test_regress/t/t_flag_instr_count_dpi_bad.pl diff --git a/Changes b/Changes index 40413b760..a839034b2 100644 --- a/Changes +++ b/Changes @@ -13,7 +13,9 @@ Verilator 4.211 devel **Minor:** -* Support unpackes array localparams in tasks/functions (#3078). [Geza Lore] +* Support unpacked array localparams in tasks/functions (#3078). [Geza Lore] +* Add --instr-count-dpi to tune assumed DPI import cost for multithreaded + model scheduling. Default value changed to 200 (#3068). [Yinan Xu] * Output files are split based on the set of headers required in order to aid incremental compilation via ccache (#3071). [Geza Lore] * Parameter values are now emitted as 'static constexpr' instead of enum. diff --git a/bin/verilator b/bin/verilator index b8d272440..0a9ceaf71 100755 --- a/bin/verilator +++ b/bin/verilator @@ -333,6 +333,7 @@ detailed descriptions of these arguments. --if-depth Tune IFDEPTH warning +incdir+ Directory to search for includes --inline-mult Tune module inlining + --instr-count-dpi Assumed dynamic instruction count of DPI imports -LDFLAGS Linker pre-object arguments for makefile --l2-name Verilog scope name of the top module --language Default language standard to parse diff --git a/docs/guide/exe_verilator.rst b/docs/guide/exe_verilator.rst index f92024741..c12214f0f 100644 --- a/docs/guide/exe_verilator.rst +++ b/docs/guide/exe_verilator.rst @@ -536,6 +536,14 @@ Summary: times, but potentially faster simulation speed. This setting is ignored for very small modules; they will always be inlined, if allowed. +.. option:: --instr-count-dpi + + Assumed dynamic instruction count of the average DPI import. This is used + by the partitioning algorithm when creating a multithread model. The + default value is 200. Adjusting this to an appropriate value can yield + performance improvements in multithreaded models. Ignored when creating a + single threaded model. + .. option:: -j [] Specify the level of parallelism for :vlopt:`--build`. The must @@ -1044,6 +1052,8 @@ Summary: Verilator assumes DPI pure imports are threadsafe, but non-pure DPI imports are not. + See also :vlopt:`--instr-count-dpi` option. + .. option:: --threads-max-mtasks Rarely needed. When using :vlopt:`--threads`, specify the number of diff --git a/docs/guide/verilating.rst b/docs/guide/verilating.rst index d1231a314..b33dd4acd 100644 --- a/docs/guide/verilating.rst +++ b/docs/guide/verilating.rst @@ -216,6 +216,12 @@ be done by a "main thread". In most cases the eval thread and main thread are the same thread (i.e. the user's top C++ testbench runs on a single thread), but this is not required. +When making frequent use of DPI imported functions in a multi-threaded +model, it may be beneficial to performance to adjust the +:vlopt:`--instr-count-dpi` option based on some experimentation. This +influences the partitioning of the model by adjusting the assumed execution +time of DPI imports. + The :vlopt:`--trace-threads` options can be used to produce trace dumps using multiple threads. If :vlopt:`--trace-threads` is set without :vlopt:`--threads`, then :vlopt:`--trace-threads` will imply diff --git a/src/V3Ast.h b/src/V3Ast.h index 63d86ae53..13e4f5020 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -1524,7 +1524,6 @@ public: static constexpr int INSTR_COUNT_STR = 100; // String ops static constexpr int INSTR_COUNT_TIME = INSTR_COUNT_CALL + 5; // Determine simulation time static constexpr int INSTR_COUNT_PLI = 20; // PLI routines - static constexpr int INSTR_COUNT_DPI = 1000; // DPI import function // ACCESSORS virtual string name() const { return ""; } diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 6fb566b7f..cdf54bd7a 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -8788,7 +8788,9 @@ public: } // virtual void name(const string& name) override { m_name = name; } - virtual int instrCount() const override { return dpiImportPrototype() ? INSTR_COUNT_DPI : 0; } + virtual int instrCount() const override { + return dpiImportPrototype() ? v3Global.opt.instrCountDpi() : 0; + } VBoolOrUnknown isConst() const { return m_isConst; } void isConst(bool flag) { m_isConst.setTrueOrFalse(flag); } void isConst(VBoolOrUnknown flag) { m_isConst = flag; } diff --git a/src/V3Options.cpp b/src/V3Options.cpp index 6677c795c..6b4fc0207 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -1101,6 +1101,10 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, char DECL_OPTION("-if-depth", Set, &m_ifDepth); DECL_OPTION("-ignc", OnOff, &m_ignc); DECL_OPTION("-inline-mult", Set, &m_inlineMult); + DECL_OPTION("-instr-count-dpi", CbVal, [this, fl](int val) { + m_instrCountDpi = val; + if (m_instrCountDpi < 0) fl->v3fatal("--instr-count-dpi must be non-negative: " << val); + }); DECL_OPTION("-LDFLAGS", CbVal, callStrSetter(&V3Options::addLdLibs)); const auto setLang = [this, fl](const char* valp) { diff --git a/src/V3Options.h b/src/V3Options.h index 6d42ccabe..acea42e08 100644 --- a/src/V3Options.h +++ b/src/V3Options.h @@ -289,6 +289,7 @@ private: int m_gateStmts = 100; // main switch: --gate-stmts int m_ifDepth = 0; // main switch: --if-depth int m_inlineMult = 2000; // main switch: --inline-mult + int m_instrCountDpi = 200; // main switch: --instr-count-dpi VOptionBool m_makeDepend; // main switch: -MMD int m_maxNumWidth = 65536; // main switch: --max-num-width int m_moduleRecursion = 100; // main switch: --module-recursion-depth @@ -489,6 +490,7 @@ public: int gateStmts() const { return m_gateStmts; } int ifDepth() const { return m_ifDepth; } int inlineMult() const { return m_inlineMult; } + int instrCountDpi() const { return m_instrCountDpi; } VOptionBool makeDepend() const { return m_makeDepend; } int maxNumWidth() const { return m_maxNumWidth; } int moduleRecursionDepth() const { return m_moduleRecursion; } diff --git a/test_regress/t/t_flag_instr_count_dpi_bad.pl b/test_regress/t/t_flag_instr_count_dpi_bad.pl new file mode 100755 index 000000000..33b76e374 --- /dev/null +++ b/test_regress/t/t_flag_instr_count_dpi_bad.pl @@ -0,0 +1,20 @@ +#!/usr/bin/env 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. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt => 1); + +compile( + verilator_flags2 => ["--instr-count-dpi -1"], + fails => 1, + expect => "%Error: --instr-count-dpi must be non-negative: -1" + ); + +ok(1); +1;