From d87ef8394ac7490f7dff3c093c2f56fd921f7388 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 30 Nov 2022 19:42:21 -0500 Subject: [PATCH] Fix CASEINCOMPLETE when covers all enum values (#3745) (#3782). Co-authored-by: "G-A. Kamendje" --- Changes | 1 + docs/CONTRIBUTORS | 1 + docs/guide/warnings.rst | 6 +++ src/V3Case.cpp | 45 +++++++++++++++---- test_regress/t/t_case_enum_complete.pl | 19 ++++++++ test_regress/t/t_case_enum_complete.v | 20 +++++++++ test_regress/t/t_case_enum_incomplete_bad.out | 6 +++ test_regress/t/t_case_enum_incomplete_bad.pl | 20 +++++++++ test_regress/t/t_case_enum_incomplete_bad.v | 19 ++++++++ 9 files changed, 129 insertions(+), 8 deletions(-) create mode 100755 test_regress/t/t_case_enum_complete.pl create mode 100644 test_regress/t/t_case_enum_complete.v create mode 100644 test_regress/t/t_case_enum_incomplete_bad.out create mode 100755 test_regress/t/t_case_enum_incomplete_bad.pl create mode 100644 test_regress/t/t_case_enum_incomplete_bad.v diff --git a/Changes b/Changes index 5e6743357..3dfd580ca 100644 --- a/Changes +++ b/Changes @@ -23,6 +23,7 @@ Verilator 5.003 devel * Internal AST improvements, also affect XML format (#3721). [Geza Lore] * Change ENDLABEL from warning into an error. * Deprecate verilated_fst_sc.cpp and verilated_vcd_sc.cpp. +* Fix CASEINCOMPLETE when covers all enum values (#3745) (#3782). [Guy-Armand Kamendje] * Fix return type of $countbits functions to int (#3725). [Ryszard Rozak, Antmicro Ltd] * Fix missing UNUSED warnings with --coverage (#3736). [alejandro-castro-ortegon] * Fix tracing parameters overridden with -G (#3723). [Iztok Jeras] diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index 517d03f61..29af74174 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -36,6 +36,7 @@ Glen Gibb Graham Rushton Guokai Chen Gustav Svensk +G-A. Kamendje Harald Heckmann Howard Su Huang Rui diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index b91ed900a..071fc01b8 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -253,6 +253,12 @@ List Of Warnings :code:`default: ;` so that any design assumption violations will be discovered in simulation. + Unique case statements that select on an enumerated variable, where all + of the enumerated values are covered by case items, are considered + complete even if illegal non-enumerated values are not covered by the + case statement. To check that illegal values are not hit, use + :vlopt:`--assert` (see IEEE 1800-2017 12.5.3). + Ignoring this warning will only suppress the lint check, it will simulate correctly. diff --git a/src/V3Case.cpp b/src/V3Case.cpp index a8ffe9ae0..dc378d51f 100644 --- a/src/V3Case.cpp +++ b/src/V3Case.cpp @@ -139,7 +139,33 @@ private: std::array m_valueItem; // METHODS - + bool caseIsEnumComplete(AstCase* nodep, uint32_t numCases) { + // Return true if case is across an enum, and every value in the case + // statement corresponds to one of the enum values + if (!nodep->uniquePragma() && !nodep->unique0Pragma()) return false; + AstEnumDType* const enumDtp + = VN_CAST(nodep->exprp()->dtypep()->skipRefToEnump(), EnumDType); + if (!enumDtp) return false; // Case isn't enum + AstBasicDType* const basicp = enumDtp->subDTypep()->basicp(); + if (!basicp) return false; // Not simple type (perhaps IEEE illegal) + if (basicp->width() > 32) return false; + // Find all case values into a set + std::set caseSet; + for (uint32_t i = 0; i < numCases; ++i) { // All case items + if (m_valueItem[i]) caseSet.emplace(i); + } + // Find all enum values into a set + std::set enumSet; + for (AstEnumItem* itemp = enumDtp->itemsp(); itemp; + itemp = VN_AS(itemp->nextp(), EnumItem)) { + AstConst* const econstp = VN_AS(itemp->valuep(), Const); + const uint32_t val = econstp->toUInt(); + // UINFO(9, "Complete enum item " << val << ": " << itemp << endl); + enumSet.emplace(val); + } + // If sets match, all covered + return (caseSet == enumSet); + } bool isCaseTreeFast(AstCase* nodep) { int width = 0; bool opaque = false; @@ -227,15 +253,18 @@ private: } } } - for (uint32_t i = 0; i < numCases; ++i) { - if (!m_valueItem[i]) { - nodep->v3warn(CASEINCOMPLETE, "Case values incompletely covered " - "(example pattern 0x" - << std::hex << i << ")"); - m_caseNoOverlapsAllCovered = false; - return false; + if (!caseIsEnumComplete(nodep, numCases)) { + for (uint32_t i = 0; i < numCases; ++i) { + if (!m_valueItem[i]) { + nodep->v3warn(CASEINCOMPLETE, "Case values incompletely covered " + "(example pattern 0x" + << std::hex << i << ")"); + m_caseNoOverlapsAllCovered = false; + return false; + } } } + if (m_caseItems <= 3 // Avoid e.g. priority expanders from going crazy in expansion || (m_caseWidth >= 8 && (m_caseItems <= (m_caseWidth + 1)))) { diff --git a/test_regress/t/t_case_enum_complete.pl b/test_regress/t/t_case_enum_complete.pl new file mode 100755 index 000000000..c2379584a --- /dev/null +++ b/test_regress/t/t_case_enum_complete.pl @@ -0,0 +1,19 @@ +#!/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(linter => 1); + +lint( + verilator_flags2 => ["--lint-only -Wwarn-CASEINCOMPLETE"], + fails => 0, + ); + +ok(1); +1; diff --git a/test_regress/t/t_case_enum_complete.v b/test_regress/t/t_case_enum_complete.v new file mode 100644 index 000000000..7ee354cfd --- /dev/null +++ b/test_regress/t/t_case_enum_complete.v @@ -0,0 +1,20 @@ +// DESCRIPTION: Verilator: SystemVerilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module t; + + enum logic [2:0] {S0, S1, S2} state; + + initial begin + state = S1; + + unique case (state) + S0: $stop; + S1: $finish; + S2: $stop; + endcase + end +endmodule diff --git a/test_regress/t/t_case_enum_incomplete_bad.out b/test_regress/t/t_case_enum_incomplete_bad.out new file mode 100644 index 000000000..4a88a83dc --- /dev/null +++ b/test_regress/t/t_case_enum_incomplete_bad.out @@ -0,0 +1,6 @@ +%Warning-CASEINCOMPLETE: t/t_case_enum_incomplete_bad.v:14:14: Case values incompletely covered (example pattern 0x1) + 14 | unique case (state) + | ^~~~ + ... For warning description see https://verilator.org/warn/CASEINCOMPLETE?v=latest + ... Use "/* verilator lint_off CASEINCOMPLETE */" and lint_on around source to disable this message. +%Error: Exiting due to diff --git a/test_regress/t/t_case_enum_incomplete_bad.pl b/test_regress/t/t_case_enum_incomplete_bad.pl new file mode 100755 index 000000000..1d5773686 --- /dev/null +++ b/test_regress/t/t_case_enum_incomplete_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 2022 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( + verilator_flags2 => ['--assert'], + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_case_enum_incomplete_bad.v b/test_regress/t/t_case_enum_incomplete_bad.v new file mode 100644 index 000000000..b6f6bc740 --- /dev/null +++ b/test_regress/t/t_case_enum_incomplete_bad.v @@ -0,0 +1,19 @@ +// DESCRIPTION: Verilator: SystemVerilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module t; + + enum logic [2:0] {S0, S1, S2} state; + + initial begin + state = S1; + + unique case (state) + S0: $stop; + S2: $stop; + endcase + end +endmodule