From 173f57c63639d5b4be2a229c468c84ffbfc0ecf8 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Fri, 3 Jun 2022 19:41:59 -0400 Subject: [PATCH] Changed --no-merge-const-pool to -fno-merge-const-pool (#3436). --- Changes | 1 + bin/verilator | 2 +- docs/guide/exe_verilator.rst | 15 ++++---- src/V3OptionParser.cpp | 34 ++++++++++++++++--- src/V3OptionParser.h | 16 ++++++--- src/V3Options.cpp | 2 +- src/V3Options.h | 4 +-- src/V3Premit.cpp | 2 +- .../t/t_extract_static_const_no_merge.pl | 2 +- 9 files changed, 56 insertions(+), 22 deletions(-) diff --git a/Changes b/Changes index 8c9418c9f..ec20c6fda 100644 --- a/Changes +++ b/Changes @@ -17,6 +17,7 @@ Verilator 4.223 devel **Minor:** +* Changed --no-merge-const-pool to -fno-merge-const-pool (#3436). * Support compile time trace signal selection with tracing_on/off (#3323). [Shunyao CAD] * Add assert when VerilatedContext is mis-deleted (#3121). [Rupert Swarbrick] * Define VM_TRACE_VCD when tracing in VCD format. [Geza Lore, Shunyao CAD] diff --git a/bin/verilator b/bin/verilator index 40be6ba0f..367651d32 100755 --- a/bin/verilator +++ b/bin/verilator @@ -319,6 +319,7 @@ detailed descriptions of these arguments. -f Parse arguments from a file -FI Force include of a file --flatten Force inlining of all modules, tasks and functions + --fno-merge-const-pool Disable merging of different types in const pool -G= Overwrite top-level parameter --gdb Run Verilator under GDB interactively --gdbbt Run Verilator under GDB for backtrace @@ -344,7 +345,6 @@ detailed descriptions of these arguments. --MMD Create .d dependency files --MP Create phony dependency targets --Mdir Name of output object directory - --no-merge-const-pool Disable merging of different types in const pool --mod-prefix Name to prepend to lower classes --no-clk Prevent marking specified signal as clock --no-decoration Disable comments and symbol decorations diff --git a/docs/guide/exe_verilator.rst b/docs/guide/exe_verilator.rst index 6100dcd55..70b3752ad 100644 --- a/docs/guide/exe_verilator.rst +++ b/docs/guide/exe_verilator.rst @@ -431,6 +431,14 @@ Summary: flattening large designs may require significant CPU time, memory and storage. +.. option:: --fno-merge-const-pool + + Rarely needed; only use if recommended by maintainers. In order to + minimize cache footprint, values of different data type, that are yet + emitted identically in C++ are merged in the constant pool. This option + disables this and causes every constant pool entry with a distinct data + type to be emitted separately. + .. option:: -G= Overwrites the given parameter of the toplevel module. The value is @@ -648,13 +656,6 @@ Summary: The directory is created if it does not exist and the parent directories exist; otherwise manually create the Mdir before calling Verilator. -.. option:: --no-merge-const-pool - - Rarely needed. In order to minimize cache footprint, values of different - data type, that are yet emitted identically in C++ are merged in the - constant pool. This option disables this and causes every constant pool - entry with a distinct data type to be emitted separately. - .. option:: --mod-prefix Specifies the name to prepend to all lower level classes. Defaults to diff --git a/src/V3OptionParser.cpp b/src/V3OptionParser.cpp index 4439ba53d..d98b4fd90 100644 --- a/src/V3OptionParser.cpp +++ b/src/V3OptionParser.cpp @@ -30,6 +30,7 @@ struct V3OptionParser::Impl { // Setting for isOnOffAllowed() and isPartialMatchAllowed() enum class en : uint8_t { NONE, // "-opt" + FONOFF, // "-fopt" and "-fno-opt" ONOFF, // "-opt" and "-no-opt" VALUE // "-opt val" }; @@ -39,6 +40,7 @@ struct V3OptionParser::Impl { bool m_undocumented = false; // This option is not documented public: virtual bool isValueNeeded() const override final { return MODE == en::VALUE; } + virtual bool isFOnOffAllowed() const override final { return MODE == en::FONOFF; } virtual bool isOnOffAllowed() const override final { return MODE == en::ONOFF; } virtual bool isPartialMatchAllowed() const override final { return ALLOW_PARTIAL_MATCH; } virtual bool isUndocumented() const override { return m_undocumented; } @@ -47,6 +49,7 @@ struct V3OptionParser::Impl { // Actual action classes template class ActionSet; // "-opt" for bool-ish, "-opt val" for int and string + template class ActionFOnOff; // "-fopt" and "-fno-opt" for bool-ish template class ActionOnOff; // "-opt" and "-no-opt" for bool-ish class ActionCbCall; // Callback without argument for "-opt" class ActionCbOnOff; // Callback for "-opt" and "-no-opt" @@ -80,6 +83,7 @@ V3OPTION_PARSER_DEF_ACT_CLASS(ActionSet, VOptionBool, m_valp->setTrueOrFalse(tru V3OPTION_PARSER_DEF_ACT_CLASS(ActionSet, int, *m_valp = std::atoi(argp), en::VALUE); V3OPTION_PARSER_DEF_ACT_CLASS(ActionSet, string, *m_valp = argp, en::VALUE); +V3OPTION_PARSER_DEF_ACT_CLASS(ActionFOnOff, bool, *m_valp = !hasPrefixFNo(optp), en::FONOFF); V3OPTION_PARSER_DEF_ACT_CLASS(ActionOnOff, bool, *m_valp = !hasPrefixNo(optp), en::ONOFF); #ifndef V3OPTION_PARSER_NO_VOPTION_BOOL V3OPTION_PARSER_DEF_ACT_CLASS(ActionOnOff, VOptionBool, m_valp->setTrueOrFalse(!hasPrefixNo(optp)), @@ -117,12 +121,23 @@ V3OPTION_PARSER_DEF_ACT_CB_CLASS(ActionCbPartialMatchVal, void(const char*, cons V3OptionParser::ActionIfs* V3OptionParser::find(const char* optp) { const auto it = m_pimpl->m_options.find(optp); - if (it != m_pimpl->m_options.end()) return it->second.get(); + if (it != m_pimpl->m_options.end()) return it->second.get(); // Exact match for (auto&& act : m_pimpl->m_options) { + if (act.second->isFOnOffAllowed()) { // Find starts with "-fno" + if (const char* const nop + = VString::startsWith(optp, "-fno-") ? (optp + strlen("-fno-")) : nullptr) { + if (act.first.substr(strlen("-f"), std::string::npos) + == nop) { // [-f]opt = [-fno-]opt + return act.second.get(); + } + } + } if (act.second->isOnOffAllowed()) { // Find starts with "-no" - const char* const nop = VString::startsWith(optp, "-no") ? (optp + 3) : nullptr; - if (nop && (act.first == nop || act.first == (string{"-"} + nop))) { - return act.second.get(); + if (const char* const nop + = VString::startsWith(optp, "-no") ? (optp + strlen("-no")) : nullptr) { + if (act.first == nop || act.first == (string{"-"} + nop)) { + return act.second.get(); + } } } else if (act.second->isPartialMatchAllowed()) { if (VString::startsWith(optp, act.first)) return act.second.get(); @@ -143,6 +158,12 @@ V3OptionParser::ActionIfs& V3OptionParser::add(const std::string& opt, ARG arg) return *insertedResult.first->second; } +bool V3OptionParser::hasPrefixFNo(const char* strp) { + UASSERT(strp[0] == '-', strp << " does not start with '-'"); + if (strp[1] == '-') ++strp; + return VString::startsWith(strp, "-fno"); +} + bool V3OptionParser::hasPrefixNo(const char* strp) { UASSERT(strp[0] == '-', strp << " does not start with '-'"); if (strp[1] == '-') ++strp; @@ -178,6 +199,10 @@ void V3OptionParser::finalize() { for (auto&& opt : m_pimpl->m_options) { if (opt.second->isUndocumented()) continue; m_pimpl->m_spellCheck.pushCandidate(opt.first); + if (opt.second->isFOnOffAllowed()) { + m_pimpl->m_spellCheck.pushCandidate( + "-fno-" + opt.first.substr(strlen("-f"), std::string::npos)); + } if (opt.second->isOnOffAllowed()) m_pimpl->m_spellCheck.pushCandidate("-no" + opt.first); } m_pimpl->m_isFinalized = true; @@ -202,6 +227,7 @@ V3OPTION_PARSER_DEF_OP(Set, VOptionBool*, ActionSet) #endif V3OPTION_PARSER_DEF_OP(Set, int*, ActionSet) V3OPTION_PARSER_DEF_OP(Set, string*, ActionSet) +V3OPTION_PARSER_DEF_OP(FOnOff, bool*, ActionFOnOff) V3OPTION_PARSER_DEF_OP(OnOff, bool*, ActionOnOff) #ifndef V3OPTION_PARSER_NO_VOPTION_BOOL V3OPTION_PARSER_DEF_OP(OnOff, VOptionBool*, ActionOnOff) diff --git a/src/V3OptionParser.h b/src/V3OptionParser.h index fc199264f..e77f43a26 100644 --- a/src/V3OptionParser.h +++ b/src/V3OptionParser.h @@ -66,6 +66,7 @@ private: // METHODS ActionIfs* find(const char* optp); template ActionIfs& add(const string& opt, ARG arg); + static bool hasPrefixFNo(const char* strp); // Returns true if strp starts with "-fno" static bool hasPrefixNo(const char* strp); // Returns true if strp starts with "-no" public: @@ -87,6 +88,7 @@ class V3OptionParser::ActionIfs VL_NOT_FINAL { public: virtual ~ActionIfs() = default; virtual bool isValueNeeded() const = 0; // Need val of "-opt val" + virtual bool isFOnOffAllowed() const = 0; // true if "-fno-opt" is allowd virtual bool isOnOffAllowed() const = 0; // true if "-no-opt" is allowd virtual bool isPartialMatchAllowed() const = 0; // true if "-Wno-" matches "-Wno-fatal" virtual bool isUndocumented() const = 0; // Will not be suggested in typo @@ -101,13 +103,15 @@ class V3OptionParser::AppendHelper final { public: // TYPES // Tag to specify which operator() to call - struct Set {}; // For ActionSet + struct FOnOff {}; // For ActionFOnOff struct OnOff {}; // For ActionOnOff + struct Set {}; // For ActionSet + struct CbCall {}; // For ActionCbCall - struct CbOnOff {}; // For ActionOnOff - struct CbVal {}; // For ActionCbVal + struct CbOnOff {}; // For ActionOnOff of ActionFOnOff struct CbPartialMatch {}; // For ActionCbPartialMatch struct CbPartialMatchVal {}; // For ActionCbPartialMatchVal + struct CbVal {}; // For ActionCbVal private: // MEMBERS @@ -122,6 +126,7 @@ public: ActionIfs& operator()(const char* optp, Set, int*) const; ActionIfs& operator()(const char* optp, Set, string*) const; + ActionIfs& operator()(const char* optp, FOnOff, bool*) const; ActionIfs& operator()(const char* optp, OnOff, bool*) const; #ifndef V3OPTION_PARSER_NO_VOPTION_BOOL ActionIfs& operator()(const char* optp, OnOff, VOptionBool*) const; @@ -144,13 +149,14 @@ public: #define V3OPTION_PARSER_DECL_TAGS \ const auto Set VL_ATTR_UNUSED = V3OptionParser::AppendHelper::Set{}; \ + const auto FOnOff VL_ATTR_UNUSED = V3OptionParser::AppendHelper::FOnOff{}; \ const auto OnOff VL_ATTR_UNUSED = V3OptionParser::AppendHelper::OnOff{}; \ const auto CbCall VL_ATTR_UNUSED = V3OptionParser::AppendHelper::CbCall{}; \ const auto CbOnOff VL_ATTR_UNUSED = V3OptionParser::AppendHelper::CbOnOff{}; \ - const auto CbVal VL_ATTR_UNUSED = V3OptionParser::AppendHelper::CbVal{}; \ const auto CbPartialMatch VL_ATTR_UNUSED = V3OptionParser::AppendHelper::CbPartialMatch{}; \ const auto CbPartialMatchVal VL_ATTR_UNUSED \ - = V3OptionParser::AppendHelper::CbPartialMatchVal {} + = V3OptionParser::AppendHelper::CbPartialMatchVal{}; \ + const auto CbVal VL_ATTR_UNUSED = V3OptionParser::AppendHelper::CbVal{}; //###################################################################### diff --git a/src/V3Options.cpp b/src/V3Options.cpp index 93d23eb5e..2a4c3050d 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -1082,6 +1082,7 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, char parseOptsFile(fl, parseFileArg(optdir, valp), false); }); DECL_OPTION("-flatten", OnOff, &m_flatten); + DECL_OPTION("-fmerge-const-pool", FOnOff, &m_fMergeConstPool); DECL_OPTION("-G", CbPartialMatch, [this](const char* optp) { addParameter(optp, false); }); DECL_OPTION("-gate-stmts", Set, &m_gateStmts); @@ -1152,7 +1153,6 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, char } }); DECL_OPTION("-max-num-width", Set, &m_maxNumWidth); - DECL_OPTION("-merge-const-pool", OnOff, &m_mergeConstPool); DECL_OPTION("-mod-prefix", Set, &m_modPrefix); DECL_OPTION("-O", CbPartialMatch, [this](const char* optp) { diff --git a/src/V3Options.h b/src/V3Options.h index b9b5ef8ff..e1756ab3d 100644 --- a/src/V3Options.h +++ b/src/V3Options.h @@ -239,6 +239,7 @@ private: bool m_dumpDefines = false; // main switch: --dump-defines bool m_dumpTreeAddrids = false; // main switch: --dump-tree-addrids bool m_exe = false; // main switch: --exe + bool m_fMergeConstPool = true; // main switch: --fmerge-const-pool bool m_flatten = false; // main switch: --flatten bool m_hierarchical = false; // main switch: --hierarchical bool m_hierChild = false; // main switch: --hierarchical-child @@ -246,7 +247,6 @@ private: bool m_lintOnly = false; // main switch: --lint-only bool m_gmake = false; // main switch: --make gmake bool m_main = false; // main swithc: --main - bool m_mergeConstPool = true; // main switch: --merge-const-pool bool m_orderClockDly = true; // main switch: --order-clock-delay bool m_outFormatOk = false; // main switch: --cc, --sc or --sp was specified bool m_pedantic = false; // main switch: --Wpedantic @@ -448,6 +448,7 @@ public: bool dpiHdrOnly() const { return m_dpiHdrOnly; } bool dumpDefines() const { return m_dumpDefines; } bool exe() const { return m_exe; } + bool fMergeConstPool() const { return m_fMergeConstPool; } bool flatten() const { return m_flatten; } bool gmake() const { return m_gmake; } bool threadsDpiPure() const { return m_threadsDpiPure; } @@ -459,7 +460,6 @@ public: bool traceStructs() const { return m_traceStructs; } bool traceUnderscore() const { return m_traceUnderscore; } bool main() const { return m_main; } - bool mergeConstPool() const { return m_mergeConstPool; } bool orderClockDly() const { return m_orderClockDly; } bool outFormatOk() const { return m_outFormatOk; } bool keepTempFiles() const { return (V3Error::debugDefault() != 0); } diff --git a/src/V3Premit.cpp b/src/V3Premit.cpp index 7501cd456..836b7c814 100644 --- a/src/V3Premit.cpp +++ b/src/V3Premit.cpp @@ -133,7 +133,7 @@ private: && !constp->num().isString(); // Not a string if (useConstPool) { // Extract into constant pool. - const bool merge = v3Global.opt.mergeConstPool(); + const bool merge = v3Global.opt.fMergeConstPool(); varp = v3Global.rootp()->constPoolp()->findConst(constp, merge)->varp(); nodep->deleteTree(); ++m_extractedToConstPool; diff --git a/test_regress/t/t_extract_static_const_no_merge.pl b/test_regress/t/t_extract_static_const_no_merge.pl index ff9a694d4..f656fe455 100755 --- a/test_regress/t/t_extract_static_const_no_merge.pl +++ b/test_regress/t/t_extract_static_const_no_merge.pl @@ -14,7 +14,7 @@ top_filename("t/t_extract_static_const.v"); golden_filename("t/t_extract_static_const.out"); compile( - verilator_flags2 => ["--stats", "--no-merge-const-pool"], + verilator_flags2 => ["--stats", "--fno-merge-const-pool"], ); execute(