From d1c787540623ad22fc734a18d0cc896a8ca393cd Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sat, 21 Oct 2023 13:53:56 +0100 Subject: [PATCH] Fix conditionals on obsolete --threads 0 Since we removed --threads 0 support, the 'threads()' option always returns a value >= 1. Remove corresponding dead code. Some of the coverage counters appear to use atomics even if the model is single threaded. I'm under the impression this was a bug originally so those ones I changed to use threads() > 1 instead. --- src/V3EmitCFunc.h | 2 +- src/V3EmitCHeaders.cpp | 2 +- src/V3EmitCImp.cpp | 4 ++-- src/V3EmitCMake.cpp | 4 +--- src/V3EmitCModel.cpp | 4 ++-- src/V3EmitCSyms.cpp | 2 +- src/V3EmitMk.cpp | 2 +- src/V3HierBlock.cpp | 9 ++------- src/V3Options.cpp | 8 ++++---- src/V3Options.h | 2 +- 10 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index dbf5026a0..a90368ace 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -603,7 +603,7 @@ public: puts(");\n"); } void visit(AstCoverInc* nodep) override { - if (v3Global.opt.threads()) { + if (v3Global.opt.threads() > 1) { puts("vlSymsp->__Vcoverage["); puts(cvtToStr(nodep->declp()->dataDeclThisp()->binNum())); puts("].fetch_add(1, std::memory_order_relaxed);\n"); diff --git a/src/V3EmitCHeaders.cpp b/src/V3EmitCHeaders.cpp index 8d9f71c8d..df00d1bef 100644 --- a/src/V3EmitCHeaders.cpp +++ b/src/V3EmitCHeaders.cpp @@ -170,7 +170,7 @@ class EmitCHeader final : public EmitCConstInit { if (v3Global.opt.coverage() && !VN_IS(modp, Class)) { decorateFirst(first, section); puts("void __vlCoverInsert("); - puts(v3Global.opt.threads() ? "std::atomic" : "uint32_t"); + puts(v3Global.opt.threads() > 1 ? "std::atomic" : "uint32_t"); puts("* countp, bool enable, const char* filenamep, int lineno, int column,\n"); puts("const char* hierp, const char* pagep, const char* commentp, const char* " "linescovp);\n"); diff --git a/src/V3EmitCImp.cpp b/src/V3EmitCImp.cpp index e20320052..7ec3acee9 100644 --- a/src/V3EmitCImp.cpp +++ b/src/V3EmitCImp.cpp @@ -307,12 +307,12 @@ class EmitCImp final : EmitCFunc { // function. This gets around gcc slowness constructing all of the template // arguments. puts("void " + prefixNameProtect(m_modp) + "::__vlCoverInsert("); - puts(v3Global.opt.threads() ? "std::atomic" : "uint32_t"); + puts(v3Global.opt.threads() > 1 ? "std::atomic" : "uint32_t"); puts("* countp, bool enable, const char* filenamep, int lineno, int column,\n"); puts("const char* hierp, const char* pagep, const char* commentp, const char* " "linescovp) " "{\n"); - if (v3Global.opt.threads()) { + if (v3Global.opt.threads() > 1) { puts("assert(sizeof(uint32_t) == sizeof(std::atomic));\n"); puts("uint32_t* count32p = reinterpret_cast(countp);\n"); } else { diff --git a/src/V3EmitCMake.cpp b/src/V3EmitCMake.cpp index ca548d68b..1cf10b01c 100644 --- a/src/V3EmitCMake.cpp +++ b/src/V3EmitCMake.cpp @@ -160,9 +160,7 @@ class CMakeEmitter final { if (v3Global.usesTiming()) { global.emplace_back("${VERILATOR_ROOT}/include/verilated_timing.cpp"); } - if (v3Global.opt.threads()) { - global.emplace_back("${VERILATOR_ROOT}/include/verilated_threads.cpp"); - } + global.emplace_back("${VERILATOR_ROOT}/include/verilated_threads.cpp"); if (v3Global.opt.usesProfiler()) { global.emplace_back("${VERILATOR_ROOT}/include/verilated_profiler.cpp"); } diff --git a/src/V3EmitCModel.cpp b/src/V3EmitCModel.cpp index 5abad82d7..b7caefa54 100644 --- a/src/V3EmitCModel.cpp +++ b/src/V3EmitCModel.cpp @@ -420,7 +420,7 @@ class EmitCModel final : public EmitCFunc { if (v3Global.opt.threads() == 1) { puts("Verilated::endOfThreadMTask(vlSymsp->__Vm_evalMsgQp);\n"); } - if (v3Global.opt.threads()) puts("Verilated::endOfEval(vlSymsp->__Vm_evalMsgQp);\n"); + puts("Verilated::endOfEval(vlSymsp->__Vm_evalMsgQp);\n"); if (v3Global.opt.profExec()) puts("VL_EXEC_TRACE_ADD_RECORD(vlSymsp).evalEnd();\n"); puts("}\n"); @@ -482,7 +482,7 @@ class EmitCModel final : public EmitCFunc { puts("const char* " + topClassName() + "::modelName() const { return \"" + topClassName() + "\"; }\n"); puts("unsigned " + topClassName() + "::threads() const { return " - + cvtToStr(std::max(1, v3Global.opt.threads())) + "; }\n"); + + cvtToStr(v3Global.opt.threads()) + "; }\n"); puts("void " + topClassName() + "::prepareClone() const { contextp()->prepareClone(); }\n"); puts("void " + topClassName() + "::atClone() const {\n"); diff --git a/src/V3EmitCSyms.cpp b/src/V3EmitCSyms.cpp index 8ac652a4b..85ccaae23 100644 --- a/src/V3EmitCSyms.cpp +++ b/src/V3EmitCSyms.cpp @@ -489,7 +489,7 @@ void EmitCSyms::emitSymHdr() { if (m_coverBins) { puts("\n// COVERAGE\n"); - puts(v3Global.opt.threads() ? "std::atomic" : "uint32_t"); + puts(v3Global.opt.threads() > 1 ? "std::atomic" : "uint32_t"); puts(" __Vcoverage["); puts(cvtToStr(m_coverBins)); puts("];\n"); diff --git a/src/V3EmitMk.cpp b/src/V3EmitMk.cpp index e55ff26f5..8bea8b362 100644 --- a/src/V3EmitMk.cpp +++ b/src/V3EmitMk.cpp @@ -106,7 +106,7 @@ public: } if (v3Global.usesProbDist()) putMakeClassEntry(of, "verilated_probdist.cpp"); if (v3Global.usesTiming()) putMakeClassEntry(of, "verilated_timing.cpp"); - if (v3Global.opt.threads()) putMakeClassEntry(of, "verilated_threads.cpp"); + putMakeClassEntry(of, "verilated_threads.cpp"); if (v3Global.opt.usesProfiler()) { putMakeClassEntry(of, "verilated_profiler.cpp"); } diff --git a/src/V3HierBlock.cpp b/src/V3HierBlock.cpp index f1624493b..b21a6e099 100644 --- a/src/V3HierBlock.cpp +++ b/src/V3HierBlock.cpp @@ -159,7 +159,7 @@ V3StringList V3HierBlock::commandArgs(bool forCMake) const { opts.push_back(" --lib-create " + modp()->name()); // possibly mangled name if (v3Global.opt.protectKeyProvided()) opts.push_back(" --protect-key " + v3Global.opt.protectKeyDefaulted()); - opts.push_back(" --hierarchical-child " + cvtToStr(std::max(1, v3Global.opt.threads()))); + opts.push_back(" --hierarchical-child " + cvtToStr(v3Global.opt.threads())); const StrGParams gparamsStr = stringifyParams(gparams(), true); for (StrGParams::const_iterator paramIt = gparamsStr.begin(); paramIt != gparamsStr.end(); @@ -227,9 +227,6 @@ void V3HierBlock::writeCommandArgsFile(bool forCMake) const { for (const string& opt : commandOpts) *of << opt << "\n"; *of << hierBlockArgs().front() << "\n"; for (const auto& hierblockp : m_children) *of << hierblockp->hierBlockArgs().front() << "\n"; - // Hierarchical blocks should not use multi-threading, - // but needs to be thread safe when top is multi-threaded. - if (v3Global.opt.threads() > 0) *of << "--threads 1\n"; *of << v3Global.opt.allArgsStringForHierBlock(false) << "\n"; } @@ -425,9 +422,7 @@ void V3HierBlockPlan::writeCommandArgsFiles(bool forCMake) const { if (v3Global.opt.protectKeyProvided()) { *of << "--protect-key " << v3Global.opt.protectKeyDefaulted() << "\n"; } - if (v3Global.opt.threads() > 0) { - *of << "--threads " << cvtToStr(v3Global.opt.threads()) << "\n"; - } + *of << "--threads " << cvtToStr(v3Global.opt.threads()) << "\n"; *of << (v3Global.opt.systemC() ? "--sc" : "--cc") << "\n"; *of << v3Global.opt.allArgsStringForHierBlock(true) << "\n"; } diff --git a/src/V3Options.cpp b/src/V3Options.cpp index 554af3421..d64519e07 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -875,11 +875,8 @@ void V3Options::notify() VL_MT_DISABLED { } if (trace()) { - // With --trace-fst, --trace-threads implies --threads 1 unless explicitly specified - if (traceFormat().fst() && traceThreads() && !threads()) m_threads = 1; - // With --trace, --trace-threads is ignored - if (traceFormat().vcd()) m_traceThreads = threads() ? 1 : 0; + if (traceFormat().vcd()) m_traceThreads = 1; } UASSERT(!(useTraceParallel() && useTraceOffload()), @@ -906,6 +903,9 @@ void V3Options::notify() VL_MT_DISABLED { m_dumpLevel["tree"] = m_dumpLevel["tree-dot"]; } + // Sanity check of expected configuration + UASSERT(threads() >= 1, "'threads()' must return a value >= 1"); + // Preprocessor defines based on options used if (timing().isSetTrue()) V3PreShell::defineCmdLine("VERILATOR_TIMING", "1"); } diff --git a/src/V3Options.h b/src/V3Options.h index 6378c8545..259462fe8 100644 --- a/src/V3Options.h +++ b/src/V3Options.h @@ -549,7 +549,7 @@ public: int traceThreads() const { return m_traceThreads; } bool useTraceOffload() const { return trace() && traceFormat().fst() && traceThreads() > 1; } bool useTraceParallel() const { - return trace() && traceFormat().vcd() && threads() && (threads() > 1 || hierChild() > 1); + return trace() && traceFormat().vcd() && (threads() > 1 || hierChild() > 1); } bool useFstWriterThread() const { return traceThreads() && traceFormat().fst(); } unsigned vmTraceThreads() const {