diff --git a/docs/guide/exe_verilator.rst b/docs/guide/exe_verilator.rst index b49b62b73..f748f6ea1 100644 --- a/docs/guide/exe_verilator.rst +++ b/docs/guide/exe_verilator.rst @@ -2382,8 +2382,10 @@ The grammar of control commands is as follows: (or wildcard with '\*' or '?', or all files if omitted) and range of line numbers (or all lines if omitted). - With lint_off using "\*" will override any lint_on directives in the - source, i.e. the warning will still not be printed. + If a warning is disabled with lint_off, it will not be printed, even if the + source contains a lint_on metacomment. The control file directives and + metacomments are interpreted separately and do not interact. A warning is + emitted only if not disabled either in a control file or via metacomments. If the ``-rule`` is omitted, all lint warnings (see list in :vlopt:`-Wno-lint`) are enabled/disabled. This will override all later @@ -2492,9 +2494,10 @@ The grammar of control commands is as follows: :option:`--no-timing`, and code:`fork`/``join*`` blocks are converted into ``begin``/``end`` blocks. - Same as :option:`/*verilator&32;timing_on*/`, - :option:`/*verilator&32;timing_off*/` metacomments. - + Similar to :option:`/*verilator&32;timing_on*/`, + :option:`/*verilator&32;timing_off*/` metacomments, but interpreted + independtly. If either a control file, or metacommets disable timing + constructs, they will be disabled. .. t_dist_docs_style ignore tracing_on diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index 011789b24..f231e0894 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -45,6 +45,9 @@ Warnings may be disabled in multiple ways: lint_off -rule UNSIGNED -file "*/example.v" -lines 1 +Metacomments and control file directives do not interact. If a warning is +disabled by either metacomments, or a directive in a control file, it will not +be emitted. Error And Warning Format ======================== diff --git a/src/V3Control.cpp b/src/V3Control.cpp index fb0f88d09..d715cd2fc 100644 --- a/src/V3Control.cpp +++ b/src/V3Control.cpp @@ -432,7 +432,7 @@ public: for (; m_lastIgnore.it != m_ignLines.end(); ++m_lastIgnore.it) { if (m_lastIgnore.it->m_lineno > curlineno) break; // UINFO(9, " Hit " << *m_lastIgnore.it); - filelinep->warnOn(m_lastIgnore.it->m_code, m_lastIgnore.it->m_on); + filelinep->warnOnCtrl(m_lastIgnore.it->m_code, m_lastIgnore.it->m_on); } if (false && debug() >= 9) { for (IgnLines::const_iterator it = m_lastIgnore.it; it != m_ignLines.end(); ++it) { diff --git a/src/V3FileLine.cpp b/src/V3FileLine.cpp index a82699ec6..b02559de9 100644 --- a/src/V3FileLine.cpp +++ b/src/V3FileLine.cpp @@ -126,26 +126,32 @@ FileLineSingleton::msgEnSetIdx_t FileLineSingleton::addMsgEnBitSet(const MsgEnBi FileLineSingleton::msgEnSetIdx_t FileLineSingleton::defaultMsgEnIndex() VL_MT_SAFE { MsgEnBitSet msgEnBitSet; for (int i = V3ErrorCode::EC_MIN; i < V3ErrorCode::_ENUM_MAX; ++i) { - msgEnBitSet.set(i, !V3ErrorCode{i}.defaultsOff()); + // "-Wall" and the like only adjsut the code subset, so use default enablement there + msgEnBitSet.set(MsgEnBitSet::Subset::CODE, i, !V3ErrorCode{i}.defaultsOff()); + // The control file subset is only adjusted by the control files, everything enabled by + // default + msgEnBitSet.set(MsgEnBitSet::Subset::CTRL, i, true); } return addMsgEnBitSet(msgEnBitSet); } FileLineSingleton::msgEnSetIdx_t FileLineSingleton::msgEnSetBit(msgEnSetIdx_t setIdx, + MsgEnBitSet::Subset subset, size_t bitIdx, bool value) { - if (msgEn(setIdx).test(bitIdx) == value) return setIdx; + if (msgEn(setIdx).test(subset, bitIdx) == value) return setIdx; MsgEnBitSet msgEnBitSet{msgEn(setIdx)}; - msgEnBitSet.set(bitIdx, value); + msgEnBitSet.set(subset, bitIdx, value); return addMsgEnBitSet(msgEnBitSet); } FileLineSingleton::msgEnSetIdx_t FileLineSingleton::msgEnAnd(msgEnSetIdx_t lhsIdx, msgEnSetIdx_t rhsIdx) { - MsgEnBitSet msgEnBitSet{msgEn(lhsIdx)}; - msgEnBitSet &= msgEn(rhsIdx); - if (msgEnBitSet == msgEn(lhsIdx)) return lhsIdx; - if (msgEnBitSet == msgEn(rhsIdx)) return rhsIdx; - return addMsgEnBitSet(msgEnBitSet); + const MsgEnBitSet& lhs = msgEn(lhsIdx); + const MsgEnBitSet& rhs = msgEn(rhsIdx); + const MsgEnBitSet intersection{lhs, rhs}; + if (intersection == lhs) return lhsIdx; + if (intersection == rhs) return rhsIdx; + return addMsgEnBitSet(intersection); } // ###################################################################### @@ -415,12 +421,12 @@ void FileLine::warnUnusedOff(bool flag) { } bool FileLine::warnIsOff(V3ErrorCode code) const { - if (!msgEn().test(code)) return true; - if (!defaultFileLine().msgEn().test(code)) return true; // Global overrides local - if ((code.lintError() || code.styleError()) && !msgEn().test(V3ErrorCode::I_LINT)) { + if (!msgEn().enabled(code)) return true; + if (!defaultFileLine().msgEn().enabled(code)) return true; // Global overrides local + if ((code.lintError() || code.styleError()) && !msgEn().enabled(V3ErrorCode::I_LINT)) { return true; } - if ((code.unusedError()) && !msgEn().test(V3ErrorCode::I_UNUSED)) return true; + if ((code.unusedError()) && !msgEn().enabled(V3ErrorCode::I_UNUSED)) return true; return false; } diff --git a/src/V3FileLine.h b/src/V3FileLine.h index da4b6fa8c..f04ac5d11 100644 --- a/src/V3FileLine.h +++ b/src/V3FileLine.h @@ -21,6 +21,7 @@ #include "verilatedos.h" #include "V3Error.h" +#include "V3Hash.h" #include "V3LangCode.h" #include "V3Mutex.h" @@ -48,7 +49,58 @@ class FileLineSingleton final { // TYPES using fileNameIdx_t = uint16_t; // Increase width if 64K input files are not enough using msgEnSetIdx_t = uint16_t; // Increase width if 64K unique message sets are not enough - using MsgEnBitSet = std::bitset; + class MsgEnBitSet final { + std::bitset m_codeEn; // Enabeld by code directives/metacomments + std::bitset m_ctrlEn; // Enabled by control file + + public: + enum class Subset { + CODE = 0, // Selects m_codeEn, the enable bits used by in-code directives/metacomments + CTRL = 1, // Selects m_ctrlEn, the enable bits used by control files + }; + + // Create empty set + MsgEnBitSet() = default; + // Create intersection set + MsgEnBitSet(const MsgEnBitSet& a, const MsgEnBitSet& b) + : m_codeEn{a.m_codeEn & b.m_codeEn} + , m_ctrlEn{a.m_ctrlEn & b.m_ctrlEn} {} + + struct Hash final { + size_t operator()(const MsgEnBitSet& item) const { + const size_t hashCode + = std::hash>()(item.m_codeEn); + const size_t hashCtrl + = std::hash>()(item.m_ctrlEn); + V3Hash hash{static_cast(hashCode)}; + hash += static_cast(hashCtrl); + return hash.value(); + } + }; + + struct Equal final { + bool operator()(const MsgEnBitSet& a, const MsgEnBitSet& b) const { return a == b; } + }; + + bool operator==(const MsgEnBitSet& other) const { + return m_codeEn == other.m_codeEn && m_ctrlEn == other.m_ctrlEn; + } + + bool test(Subset subset, size_t code) const { + return subset == Subset::CODE ? m_codeEn.test(code) : m_ctrlEn.test(code); + } + + void set(Subset subset, size_t code, bool value) { + if (subset == Subset::CODE) { + m_codeEn.set(code, value); + } else { + m_ctrlEn.set(code, value); + } + } + + // Enabled iff enabled by both in-code dierctives/metacomments and control file + bool enabled(V3ErrorCode code) const { return m_codeEn.test(code) && m_ctrlEn.test(code); } + }; // MEMBERS V3Mutex m_mutex; // protects members @@ -57,7 +109,8 @@ class FileLineSingleton final { std::deque m_languages; // language for each filenameno // Map from flag set to the index in m_internedMsgEns for interning - std::unordered_map m_internedMsgEnIdxs VL_GUARDED_BY(m_mutex); + std::unordered_map + m_internedMsgEnIdxs VL_GUARDED_BY(m_mutex); // Interned message enablement flag sets std::vector m_internedMsgEns; @@ -87,7 +140,8 @@ class FileLineSingleton final { // Add index of default bitset msgEnSetIdx_t defaultMsgEnIndex() VL_MT_SAFE; // Set bitIdx to value in bitset at interned index setIdx, return interned index of result - msgEnSetIdx_t msgEnSetBit(msgEnSetIdx_t setIdx, size_t bitIdx, bool value); + msgEnSetIdx_t msgEnSetBit(msgEnSetIdx_t setIdx, MsgEnBitSet::Subset subset, size_t bitIdx, + bool value); // Return index to intersection set msgEnSetIdx_t msgEnAnd(msgEnSetIdx_t lhsIdx, msgEnSetIdx_t rhsIdx); // Retrieve interned bitset at given interned index. The returned reference is not persistent. @@ -312,14 +366,21 @@ public: string lineDirectiveStrg(int enterExit) const; // Turn on/off warning messages on this line. - void warnOn(V3ErrorCode code, bool flag) { +private: + void warnSet(MsgEnBitSet::Subset subset, V3ErrorCode code, bool flag) { if (code == V3ErrorCode::WIDTH) { - warnOn(V3ErrorCode::WIDTHTRUNC, flag); - warnOn(V3ErrorCode::WIDTHEXPAND, flag); - warnOn(V3ErrorCode::WIDTHXZEXPAND, flag); + warnSet(subset, V3ErrorCode::WIDTHTRUNC, flag); + warnSet(subset, V3ErrorCode::WIDTHEXPAND, flag); + warnSet(subset, V3ErrorCode::WIDTHXZEXPAND, flag); } - if (code == V3ErrorCode::E_UNSUPPORTED) warnOn(V3ErrorCode::COVERIGN, flag); - m_msgEnIdx = singleton().msgEnSetBit(m_msgEnIdx, code, flag); + if (code == V3ErrorCode::E_UNSUPPORTED) { warnSet(subset, V3ErrorCode::COVERIGN, flag); } + m_msgEnIdx = singleton().msgEnSetBit(m_msgEnIdx, subset, code, flag); + } + +public: + void warnOn(V3ErrorCode code, bool flag) { warnSet(MsgEnBitSet::Subset::CODE, code, flag); } + void warnOnCtrl(V3ErrorCode code, bool flag) { + warnSet(MsgEnBitSet::Subset::CTRL, code, flag); } void warnOff(V3ErrorCode code, bool flag) { warnOn(code, !flag); } string warnOffParse(const string& msgs, bool flag); // Returns "" if ok @@ -331,13 +392,13 @@ public: void warnResetDefault() { warnStateFrom(defaultFileLine()); } // Specific flag ACCESSORS/METHODS - bool celldefineOn() const { return msgEn().test(V3ErrorCode::I_CELLDEFINE); } + bool celldefineOn() const { return msgEn().enabled(V3ErrorCode::I_CELLDEFINE); } void celldefineOn(bool flag) { warnOn(V3ErrorCode::I_CELLDEFINE, flag); } - bool coverageOn() const { return msgEn().test(V3ErrorCode::I_COVERAGE); } + bool coverageOn() const { return msgEn().enabled(V3ErrorCode::I_COVERAGE); } void coverageOn(bool flag) { warnOn(V3ErrorCode::I_COVERAGE, flag); } - bool tracingOn() const { return msgEn().test(V3ErrorCode::I_TRACING); } + bool tracingOn() const { return msgEn().enabled(V3ErrorCode::I_TRACING); } void tracingOn(bool flag) { warnOn(V3ErrorCode::I_TRACING, flag); } - bool timingOn() const { return msgEn().test(V3ErrorCode::I_TIMING); } + bool timingOn() const { return msgEn().enabled(V3ErrorCode::I_TIMING); } void timingOn(bool flag) { warnOn(V3ErrorCode::I_TIMING, flag); } // METHODS - Global @@ -413,8 +474,12 @@ public: if (m_lastLinenoAdder != rhs.m_lastLinenoAdder) return (m_lastLinenoAdder < rhs.m_lastLinenoAdder) ? -1 : 1; if (m_lastColumn != rhs.m_lastColumn) return (m_lastColumn < rhs.m_lastColumn) ? -1 : 1; - for (size_t i = 0; i < msgEn().size(); ++i) { - if (msgEn().test(i) != rhs.msgEn().test(i)) return rhs.msgEn().test(i) ? -1 : 1; + const MsgEnBitSet& lhsMsgEn = msgEn(); + const MsgEnBitSet& rhsMsgEn = rhs.msgEn(); + for (size_t i = 0; i < V3ErrorCode::_ENUM_MAX; ++i) { + V3ErrorCode code = static_cast(i); + if (lhsMsgEn.enabled(code) != rhsMsgEn.enabled(code)) + return rhsMsgEn.enabled(code) ? -1 : 1; } // TokenNum is compared last as makes more logical sort order by file/line first if (m_tokenNum != rhs.m_tokenNum) return (m_tokenNum < rhs.m_tokenNum) ? -1 : 1; diff --git a/test_regress/t/t_vlt_timing.py b/test_regress/t/t_vlt_timing.py index fb26f1e48..7c4d12ae4 100755 --- a/test_regress/t/t_vlt_timing.py +++ b/test_regress/t/t_vlt_timing.py @@ -10,7 +10,6 @@ import vltest_bootstrap test.scenarios('simulator') -test.top_filename = "t/t_timing_off.v" test.compile(verilator_flags2=["--binary t/t_vlt_timing.vlt"]) diff --git a/test_regress/t/t_vlt_timing.v b/test_regress/t/t_vlt_timing.v new file mode 100644 index 000000000..e95e1ec99 --- /dev/null +++ b/test_regress/t/t_vlt_timing.v @@ -0,0 +1,39 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Antmicro Ltd. +// SPDX-License-Identifier: CC0-1.0 + +module t; + event e1; + event e2; + + initial begin + int x; + // verilator timing_off + #1 + fork @e1; @e2; join; + @e1 + wait(x == 4) + x = #1 8; + // verilator timing_on + if (x != 8) $stop; + if ($time != 0) $stop; + + @e2; + + @e1; + if ((e1.triggered && e2.triggered) + || (!e1.triggered && !e2.triggered)) $stop; + if ($time != 2) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end + + initial #2 ->e1; + + initial #2 ->e2; + + initial #3 $stop; // timeout + initial #1 @(e1, e2) #1 $stop; // timeout +endmodule diff --git a/test_regress/t/t_vlt_timing.vlt b/test_regress/t/t_vlt_timing.vlt index f5ed20a92..d35c3a5d6 100644 --- a/test_regress/t/t_vlt_timing.vlt +++ b/test_regress/t/t_vlt_timing.vlt @@ -6,6 +6,10 @@ `verilator_config -timing_on --file "t/t_timing_off.v" --lines 23 -timing_off -file "t/t_timing_off.v" -lines 26-34 -timing_on -file "t/t_timing_off.v" -lines 35-38 +timing_off --file "t/t_vlt_timing.v" +timing_on -file "t/t_vlt_timing.v" --lines 23 +// Bug here. This line should make no difference. +//timing_off --file "t/t_vlt_timing.v" --lines 22-24 +timing_on --file "t/t_vlt_timing.v" --lines 23 +timing_off -file "t/t_vlt_timing.v" -lines 26-34 +timing_on -file "t/t_vlt_timing.v" -lines 35-38 diff --git a/test_regress/t/t_vlt_warn.v b/test_regress/t/t_vlt_warn.v index 4de6b49f4..3de57b6be 100644 --- a/test_regress/t/t_vlt_warn.v +++ b/test_regress/t/t_vlt_warn.v @@ -20,6 +20,10 @@ module t; reg width_warn2_var_line19 = 2'b11; // Width warning - must be line 19 reg width_warn3_var_line20 = 2'b11; // Width warning - must be line 20 + // Must not turn back on warning disabled via control file + // verilator lint_on CASEINCOMPLETE + // verilator lint_on CASEX + initial begin casex (1'b1) 1'b0: $stop;