diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 94aa6b280..dc4be8451 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -3300,6 +3300,9 @@ class ConstVisitor final : public VNVisitor { if (nodep->precondsp()) { nodep->replaceWith(nodep->precondsp()); } else { + nodep->v3warn(UNUSEDLOOP, + "Loop condition is always false; body will never execute"); + nodep->fileline()->modifyWarnOff(V3ErrorCode::UNUSEDLOOP, true); nodep->unlinkFrBack(); } VL_DO_DANGLING(pushDeletep(nodep), nodep); diff --git a/src/V3Error.h b/src/V3Error.h index d81261484..5eba645b0 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -152,6 +152,7 @@ public: UNPACKED, // Unsupported unpacked UNSIGNED, // Comparison is constant due to unsigned arithmetic UNUSEDGENVAR, // No receivers for genvar + UNUSEDLOOP, // Loop is unused UNUSEDPARAM, // No receivers for parameters UNUSEDSIGNAL, // No receivers for signals USERERROR, // Elaboration time $error @@ -210,7 +211,7 @@ public: "STATICVAR", "STMTDLY", "SYMRSVDWORD", "SYNCASYNCNET", "TICKCOUNT", "TIMESCALEMOD", "UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNOPTTHREADS", - "UNPACKED", "UNSIGNED", "UNUSEDGENVAR", "UNUSEDPARAM", "UNUSEDSIGNAL", + "UNPACKED", "UNSIGNED", "UNUSEDGENVAR", "UNUSEDLOOP" ,"UNUSEDPARAM", "UNUSEDSIGNAL", "USERERROR", "USERFATAL", "USERINFO", "USERWARN", "VARHIDDEN", "WAITCONST", "WIDTH", "WIDTHCONCAT", "WIDTHEXPAND", "WIDTHTRUNC", "WIDTHXZEXPAND", "ZERODLY", "ZEROREPL", " MAX" @@ -254,12 +255,13 @@ public: || m_e == BLKSEQ || m_e == DEFPARAM || m_e == DECLFILENAME || m_e == EOFNEWLINE || m_e == GENUNNAMED || m_e == IMPORTSTAR || m_e == INCABSPATH || m_e == PINCONNECTEMPTY || m_e == PINNOCONNECT || m_e == SYNCASYNCNET - || m_e == UNDRIVEN || m_e == UNUSEDGENVAR || m_e == UNUSEDPARAM - || m_e == UNUSEDSIGNAL || m_e == VARHIDDEN); + || m_e == UNDRIVEN || m_e == UNUSEDGENVAR || m_e == UNUSEDLOOP + || m_e == UNUSEDPARAM || m_e == UNUSEDSIGNAL || m_e == VARHIDDEN); } // Warnings that are unused only bool unusedError() const VL_MT_SAFE { - return (m_e == UNUSEDGENVAR || m_e == UNUSEDPARAM || m_e == UNUSEDSIGNAL); + return (m_e == UNUSEDGENVAR || m_e == UNUSEDLOOP || m_e == UNUSEDPARAM + || m_e == UNUSEDSIGNAL); } V3ErrorCode renamedTo() const { @@ -275,7 +277,8 @@ public: return (m_e == WIDTHEXPAND || m_e == WIDTHTRUNC || m_e == WIDTHXZEXPAND); } if (other == V3ErrorCode::I_UNUSED) { - return (m_e == UNUSEDGENVAR || m_e == UNUSEDPARAM || m_e == UNUSEDSIGNAL); + return (m_e == UNUSEDGENVAR || m_e == UNUSEDLOOP || m_e == UNUSEDPARAM + || m_e == UNUSEDSIGNAL); } return false; } diff --git a/src/V3FileLine.cpp b/src/V3FileLine.cpp index 7a45be309..6c588ea01 100644 --- a/src/V3FileLine.cpp +++ b/src/V3FileLine.cpp @@ -365,6 +365,7 @@ bool FileLine::warnOff(const string& msg, bool flag) { // Backward compatibility with msg="UNUSED" if (V3ErrorCode::unusedMsg(cmsg)) { warnOff(V3ErrorCode::UNUSEDGENVAR, flag); + warnOff(V3ErrorCode::UNUSEDLOOP, flag); warnOff(V3ErrorCode::UNUSEDPARAM, flag); warnOff(V3ErrorCode::UNUSEDSIGNAL, flag); return true; @@ -394,6 +395,7 @@ void FileLine::warnStyleOff(bool flag) { void FileLine::warnUnusedOff(bool flag) { warnOff(V3ErrorCode::UNUSEDGENVAR, flag); + warnOff(V3ErrorCode::UNUSEDLOOP, flag); warnOff(V3ErrorCode::UNUSEDPARAM, flag); warnOff(V3ErrorCode::UNUSEDSIGNAL, flag); } diff --git a/src/V3Gate.cpp b/src/V3Gate.cpp index a25d7feea..ae22f9c9e 100644 --- a/src/V3Gate.cpp +++ b/src/V3Gate.cpp @@ -1312,6 +1312,18 @@ class GateUnused final { } } + static void warnUnused(const AstNode* const nodep) { + if (nodep->fileline()->warnIsOff(V3ErrorCode::UNUSEDLOOP)) return; + + if (const AstNodeProcedure* const procedurep = VN_CAST(nodep, NodeProcedure)) { + if (procedurep->stmtsp()) + procedurep->stmtsp()->foreach([](const AstWhile* const whilep) { // + whilep->v3warn(UNUSEDLOOP, "Loop is not used and will be optimized out"); + whilep->fileline()->modifyWarnOff(V3ErrorCode::UNUSEDLOOP, true); + }); + } + } + // Remove unused logic void remove() { for (V3GraphVertex *vtxp = m_graph.verticesBeginp(), *nextp; vtxp; vtxp = nextp) { @@ -1319,6 +1331,8 @@ class GateUnused final { if (GateLogicVertex* const lVtxp = vtxp->cast()) { if (!lVtxp->consumed() && lVtxp->activep()) { // activep is nullptr under cfunc AstNode* const nodep = lVtxp->nodep(); + warnUnused(nodep); + UINFO(8, " Remove unconsumed " << nodep << endl); nodep->unlinkFrBack(); VL_DO_DANGLING(nodep->deleteTree(), nodep); diff --git a/src/V3LinkJump.cpp b/src/V3LinkJump.cpp index eefcc0ec8..2f4dce61b 100644 --- a/src/V3LinkJump.cpp +++ b/src/V3LinkJump.cpp @@ -34,6 +34,7 @@ #include "V3LinkJump.h" #include "V3AstUserAllocator.h" +#include "V3Error.h" #include @@ -228,6 +229,8 @@ class LinkJumpVisitor final : public VNVisitor { void visit(AstWhile* nodep) override { // Don't need to track AstRepeat/AstFor as they have already been converted if (!m_unrollFull.isDefault()) nodep->unrollFull(m_unrollFull); + if (m_modp->hasParameterList() || m_modp->hasGParam()) + nodep->fileline()->modifyWarnOff(V3ErrorCode::UNUSEDLOOP, true); m_unrollFull = VOptionBool::OPT_DEFAULT_FALSE; VL_RESTORER(m_loopp); VL_RESTORER(m_loopInc); @@ -254,6 +257,8 @@ class LinkJumpVisitor final : public VNVisitor { AstWhile* const whilep = new AstWhile{nodep->fileline(), condp, bodyp}; if (!m_unrollFull.isDefault()) whilep->unrollFull(m_unrollFull); m_unrollFull = VOptionBool::OPT_DEFAULT_FALSE; + // No unused warning for converted AstDoWhile, as body always executes once + nodep->fileline()->modifyWarnOff(V3ErrorCode::UNUSEDLOOP, true); nodep->replaceWith(whilep); VL_DO_DANGLING(nodep->deleteTree(), nodep); if (bodyp) { diff --git a/src/V3Options.cpp b/src/V3Options.cpp index 332227c61..1dc36c7c9 100644 --- a/src/V3Options.cpp +++ b/src/V3Options.cpp @@ -1577,6 +1577,7 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, }); DECL_OPTION("-Werror-UNUSED", CbCall, []() { V3Error::pretendError(V3ErrorCode::UNUSEDGENVAR, true); + V3Error::pretendError(V3ErrorCode::UNUSEDLOOP, true); V3Error::pretendError(V3ErrorCode::UNUSEDPARAM, true); V3Error::pretendError(V3ErrorCode::UNUSEDSIGNAL, true); }); @@ -1630,6 +1631,7 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc, DECL_OPTION("-Wwarn-UNUSED", CbCall, []() { FileLine::globalWarnUnusedOff(false); V3Error::pretendError(V3ErrorCode::UNUSEDGENVAR, false); + V3Error::pretendError(V3ErrorCode::UNUSEDLOOP, false); V3Error::pretendError(V3ErrorCode::UNUSEDSIGNAL, false); V3Error::pretendError(V3ErrorCode::UNUSEDPARAM, false); }); diff --git a/test_regress/t/t_lint_misindent_bad.out b/test_regress/t/t_lint_misindent_bad.out index 145c4b9f3..6488b2c70 100644 --- a/test_regress/t/t_lint_misindent_bad.out +++ b/test_regress/t/t_lint_misindent_bad.out @@ -1,27 +1,27 @@ -%Warning-MISINDENT: t/t_lint_misindent_bad.v:14:9: Misleading indentation - 14 | $display("bad1"); +%Warning-MISINDENT: t/t_lint_misindent_bad.v:16:9: Misleading indentation + 16 | $display("bad1"); | ^~~~~~~~ - t/t_lint_misindent_bad.v:12:7: ... Expected indentation matching this earlier statement's line: - 12 | if (0) + t/t_lint_misindent_bad.v:14:7: ... Expected indentation matching this earlier statement's line: + 14 | if (0) | ^~ ... For warning description see https://verilator.org/warn/MISINDENT?v=latest ... Use "/* verilator lint_off MISINDENT */" and lint_on around source to disable this message. -%Warning-MISINDENT: t/t_lint_misindent_bad.v:20:9: Misleading indentation - 20 | $display("bad2"); +%Warning-MISINDENT: t/t_lint_misindent_bad.v:22:9: Misleading indentation + 22 | $display("bad2"); | ^~~~~~~~ - t/t_lint_misindent_bad.v:16:7: ... Expected indentation matching this earlier statement's line: - 16 | if (0) + t/t_lint_misindent_bad.v:18:7: ... Expected indentation matching this earlier statement's line: + 18 | if (0) | ^~ -%Warning-MISINDENT: t/t_lint_misindent_bad.v:24:9: Misleading indentation - 24 | $display("bad3"); +%Warning-MISINDENT: t/t_lint_misindent_bad.v:26:9: Misleading indentation + 26 | $display("bad3"); | ^~~~~~~~ - t/t_lint_misindent_bad.v:22:7: ... Expected indentation matching this earlier statement's line: - 22 | for (;0;) + t/t_lint_misindent_bad.v:24:7: ... Expected indentation matching this earlier statement's line: + 24 | for (;0;) | ^~~ -%Warning-MISINDENT: t/t_lint_misindent_bad.v:28:9: Misleading indentation - 28 | $display("bad4"); +%Warning-MISINDENT: t/t_lint_misindent_bad.v:30:9: Misleading indentation + 30 | $display("bad4"); | ^~~~~~~~ - t/t_lint_misindent_bad.v:26:7: ... Expected indentation matching this earlier statement's line: - 26 | while (0) + t/t_lint_misindent_bad.v:28:7: ... Expected indentation matching this earlier statement's line: + 28 | while (0) | ^~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_lint_misindent_bad.v b/test_regress/t/t_lint_misindent_bad.v index 5f6b6cab8..c97160497 100644 --- a/test_regress/t/t_lint_misindent_bad.v +++ b/test_regress/t/t_lint_misindent_bad.v @@ -6,6 +6,8 @@ // Do not reindent - spaces are critical to this test +// verilator lint_off UNUSEDLOOP + module t (/*AUTOARG*/); initial begin diff --git a/test_regress/t/t_lint_removed_unused_loop_bad.out b/test_regress/t/t_lint_removed_unused_loop_bad.out new file mode 100644 index 000000000..44ebc7330 --- /dev/null +++ b/test_regress/t/t_lint_removed_unused_loop_bad.out @@ -0,0 +1,38 @@ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:172:7: Loop condition is always false; body will never execute + : ... note: In instance 't.with_always' + 172 | while(0); + | ^~~~~ + ... For warning description see https://verilator.org/warn/UNUSEDLOOP?v=latest + ... Use "/* verilator lint_off UNUSEDLOOP */" and lint_on around source to disable this message. +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:155:7: Loop condition is always false; body will never execute + : ... note: In instance 't.non_parametrized_initial' + 155 | while(0); + | ^~~~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:114:7: Loop condition is always false; body will never execute + 114 | while(always_zero < 0) begin + | ^~~~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:156:7: Loop condition is always false; body will never execute + 156 | while(always_false); + | ^~~~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:157:7: Loop condition is always false; body will never execute + 157 | while(always_zero < 0); + | ^~~~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:174:7: Loop condition is always false; body will never execute + 174 | while(always_false) begin + | ^~~~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:184:7: Loop condition is always false; body will never execute + 184 | while(always_zero) begin + | ^~~~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:188:7: Loop condition is always false; body will never execute + 188 | for (int i = 0; always_zero; i++) + | ^~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:193:7: Loop condition is always false; body will never execute + 193 | for (int i = 0; i < always_zero; i++) + | ^~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:136:7: Loop is not used and will be optimized out + 136 | while(param_unused_while < always_zero) begin + | ^~~~~ +%Warning-UNUSEDLOOP: t/t_lint_removed_unused_loop_bad.v:283:7: Loop is not used and will be optimized out + 283 | while (m_2_ticked); + | ^~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_lint_removed_unused_loop_bad.pl b/test_regress/t/t_lint_removed_unused_loop_bad.pl new file mode 100755 index 000000000..d4d0ecb72 --- /dev/null +++ b/test_regress/t/t_lint_removed_unused_loop_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 2024 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(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + verilator_flags2 => ["--top-module t", "-Wall"] + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_removed_unused_loop_bad.v b/test_regress/t/t_lint_removed_unused_loop_bad.v new file mode 100644 index 000000000..7bd5b195d --- /dev/null +++ b/test_regress/t/t_lint_removed_unused_loop_bad.v @@ -0,0 +1,286 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2024 by Antmicro. +// SPDX-License-Identifier: CC0-1.0 + +// verilator lint_off BLKSEQ +// verilator lint_off DECLFILENAME + +module t(/*AUTOARG*/ + // Inputs + clk, reset_l + ); + input clk; + input reset_l; + + parametrized_initial#(.REPETITIONS(0)) parametrized_initial0(); + parametrized_initial#(.REPETITIONS(1)) parametrized_initial1(); + parametrized_initial#(.REPETITIONS(2)) parametrized_initial2(); + non_parametrized_initial non_parametrized_initial(); + + with_always with_always(.clk(clk)); + const_condition const_condition(); + loop_with_param loop_with_param(); + if_with_param if_with_param(); + clock_init_race clock_init_race(.clk(clk), .reset_l(reset_l)); +endmodule + +// module unused - no warning for any of statements inside +module unused(input clk); + reg unused_variable_while = 0; + reg unused_variable_do_while = 0; + reg unused_variable_for = 0; + const logic always_false = 0; + + always @(posedge clk) begin + while(unused_variable_while) begin + unused_variable_while <= 1; + end + + do begin + unused_variable_do_while <= 1; + end while (unused_variable_do_while); + + for (int i = 0; i < 5; i++) + begin + unused_variable_for <= 1; + end + + while(always_false) begin + $write("This will not be printed\n"); + end + + do begin + $write("This will not be printed\n"); + end while (always_false); + + for (int i = 0; always_false; i++) + begin + $write("This will not be printed\n"); + end + end +endmodule + +// no warning for loops under parametrized module +module parametrized_initial #(parameter REPETITIONS = 0); + int prints_while = 0; + int prints_do_while = 0; + + // loops with evaluation depending on REPETITIONS + initial begin + while(prints_while < REPETITIONS) begin + prints_while = prints_while + 1; + $write("Writing to console to avoid loop being optimized out\n"); + end + + while(REPETITIONS < 0) begin + $write("Writing to console to avoid loop being optimized out\n"); + end + + for (int i = 0; i < REPETITIONS; i++) begin + $write("Writing to console to avoid loop being optimized out\n"); + end + + do begin + prints_do_while = prints_do_while + 1; + $write("Writing to console to avoid loop being optimized out\n"); + end while (prints_do_while < REPETITIONS); + end + + // loop not changing variable used for output + int param_unused_while = 0; + initial begin + while(param_unused_while < REPETITIONS) begin + param_unused_while = param_unused_while + 1; + end + end + + const logic always_false = 0; + // loops with empty bodies + initial begin + while(0); + while(always_false); + while(REPETITIONS < 0); + end +endmodule + +module non_parametrized_initial; + int prints_do_while = 0; + const int always_zero = 0; + + // loops with evaluation depending on always_zero + initial begin + while(always_zero < 0) begin + $write("This will not be printed\n"); + end + + // unrolled - no warning + for (int i = 0; i < always_zero; i++) begin + $write("This will not be printed\n"); + end + + // inlined - no warning + do begin + prints_do_while = prints_do_while + 1; + $write("Writing to console to avoid loop being optimized out\n"); + end while (prints_do_while < always_zero); + end + + // loop not changing variable used for output + int param_unused_while = 0; + int param_unused_do_while = 0; + int param_unused_for = 0; + initial begin + // warning + while(param_unused_while < always_zero) begin + param_unused_while++; + end + + // unrolled - no warning + for (int i = 0; i < 5; i++) + begin + param_unused_for = 1; + end + + // inlined - no warning + do begin + param_unused_do_while = 1; + end while (param_unused_do_while > 0); + end + + const logic always_false = 0; + // loops with empty bodies - warning + initial begin + while(0); + while(always_false); + while(always_zero < 0); + + // inlined - no warning + do begin + end while(0); + + // unrolled - no warning + for (int i = 0; i < 1; i++); + end +endmodule + +// warning for all unused loops under always +module with_always(input clk); + const logic always_false = 0; + always @(posedge clk) begin + while(0); + + while(always_false) begin + $write("Test"); + end + end +endmodule + +module const_condition; + const logic always_zero = 0; + // loops with const false condition - warning + initial begin + while(always_zero) begin + $write("This will not be printed\n"); + end + + for (int i = 0; always_zero; i++) + begin + $write("This will not be printed\n"); + end + + for (int i = 0; i < always_zero; i++) + begin + $write("This will not be printed\n"); + end + + // inlined - no warning + do begin + $write("This will be printed\n"); + end while (always_zero); + end +endmodule + +// loop with param - no warning +module loop_with_param; + parameter ZERO_PARAM = 0; + int prints = 2; + + initial begin + for (int i = 0; ZERO_PARAM; i++) begin + $write("This will not be printed\n"); + end + + while (ZERO_PARAM != ZERO_PARAM) begin + $write("This will not be printed\n"); + end + + while(prints > ZERO_PARAM) begin + prints--; + end + end +endmodule + +module if_with_param; + parameter ZERO_PARAM = 0; + parameter ONE_PARAM = 1; + + initial begin + if (ZERO_PARAM) begin + // loop under false parametrized if - no warning + int prints = 0; + while(prints < 5) begin + prints++; + end + $write("Prints %d\n", prints); + end else if (!ONE_PARAM) begin + // loop under false parametrized if - no warning + int prints = 0; + while(prints < 5) begin + prints++; + end + $write("Prints %d\n", prints); + end else begin + // loop under true parametrized if - no warning + int prints = 0; + while(prints < 5) begin + prints++; + end + $write("Prints %d\n", prints); + end + end +endmodule + +module clock_init_race(input clk, input reset_l); + logic m_2_clock; + logic m_3_clock; + logic m_2_reset = reset_l; + logic m_3_reset = reset_l; + assign m_2_clock = clk; + assign m_3_clock = clk; + int m_3_counter = 0; + initial begin + $write("*-* START TEST *-*\n"); + end + + always @(posedge clk) begin + if (m_3_counter == 25) begin + $write("*-* All Finished *-*\n"); + $finish(); + end + end + + reg m_2_ticked = 1'b0; + always @(posedge m_2_clock) if (!m_2_reset) begin + m_2_ticked = 1'b1; + end + always @(negedge m_2_clock) m_2_ticked = 1'b0; + + always @(posedge m_3_clock) if (!m_3_reset) begin + $write("*-* m_3_clocked *-*\n"); + // loop empty - unused loop warning + while (m_2_ticked); + m_3_counter += 1; + end +endmodule