Allow overlaps in priority case statements (#2864)

This will still warn if a case item is completely covered by previous
items, but will no longer complain about overlaps like this:

    priority casez (foo_i)
      2'b ?1: bar_o = 3'd0;
      2'b 1?: bar_o = 3'd1;

Before, there was a warning for the second statement because the first
two patterns match 2'b11.

src/V3Case.cpp

@@ -159,11 +159,13 @@ private:
             return false;  // Too wide for analysis
         }
         UINFO(8, "Simple case statement: " << nodep << endl);
+        uint32_t numCases = 1UL << m_caseWidth;
         // Zero list of items for each value
-        for (uint32_t i = 0; i < (1UL << m_caseWidth); ++i) m_valueItem[i] = nullptr;
+        for (uint32_t i = 0; i < numCases; ++i) m_valueItem[i] = nullptr;
         // Now pick up the values for each assignment
         // We can cheat and use uint32_t's because we only support narrow case's
-        bool bitched = false;
+        bool reportedOverlap = false;
+        bool reportedSubcase = false;
         for (AstCaseItem* itemp = nodep->itemsp(); itemp;
              itemp = VN_CAST(itemp->nextp(), CaseItem)) {
             for (AstNode* icondp = itemp->condsp(); icondp; icondp = icondp->nextp()) {
@@ -179,29 +181,52 @@ private:
                     V3Number numval(itemp, iconstp->width());
                     numval.opBitsOne(iconstp->num());
                     uint32_t val = numval.toUInt();
-                    for (uint32_t i = 0; i < (1UL << m_caseWidth); ++i) {
+
+                    uint32_t firstOverlap = 0;
+                    bool foundOverlap = false;
+                    bool foundHit = false;
+                    for (uint32_t i = 0; i < numCases; ++i) {
                         if ((i & mask) == val) {
                             if (!m_valueItem[i]) {
                                 m_valueItem[i] = itemp;
-                            } else if (!itemp->ignoreOverlap() && !bitched) {
+                                foundHit = true;
-                                icondp->v3warn(CASEOVERLAP,
+                            } else if (!foundOverlap && !itemp->ignoreOverlap()) {
-                                               "Case values overlap (example pattern 0x"
+                                firstOverlap = i;
-                                                   << std::hex << i << ")");
+                                foundOverlap = true;
-                                bitched = true;
                                 m_caseNoOverlapsAllCovered = false;
                             }
                         }
                     }
+                    if (!nodep->priorityPragma()) {
+                        // If this case statement doesn't have the priority
+                        // keyword, we want to warn on any overlap.
+                        if (!reportedOverlap && foundOverlap) {
+                            icondp->v3warn(CASEOVERLAP, "Case values overlap (example pattern 0x"
+                                                            << std::hex << firstOverlap << ")");
+                            reportedOverlap = true;
+                        }
+                    } else {
+                        // If this is a priority case, we only want to complain
+                        // if every possible value for this item is already hit
+                        // by some other item. This is true if foundHit is
+                        // false.
+                        if (!reportedSubcase && !foundHit) {
+                            icondp->v3warn(CASEOVERLAP,
+                                           "Case item ignored: every matching value is covered "
+                                           "by an earlier item");
+                            reportedSubcase = true;
+                        }
+                    }
                 }
             }
             // Defaults were moved to last in the caseitem list by V3LinkDot
             if (itemp->isDefault()) {  // Case statement's default... Fill the table
-                for (uint32_t i = 0; i < (1UL << m_caseWidth); ++i) {
+                for (uint32_t i = 0; i < numCases; ++i) {
                     if (!m_valueItem[i]) m_valueItem[i] = itemp;
                 }
             }
         }
-        for (uint32_t i = 0; i < (1UL << m_caseWidth); ++i) {
+        for (uint32_t i = 0; i < numCases; ++i) {
             if (!m_valueItem[i]) {
                 nodep->v3warn(CASEINCOMPLETE, "Case values incompletely covered "
                                               "(example pattern 0x"
@@ -218,7 +243,7 @@ private:
 
         // Convert valueItem from AstCaseItem* to the expression
         // Not done earlier, as we may now have a nullptr because it's just a ";" NOP branch
-        for (uint32_t i = 0; i < (1UL << m_caseWidth); ++i) {
+        for (uint32_t i = 0; i < numCases; ++i) {
             m_valueItem[i] = VN_CAST(m_valueItem[i], CaseItem)->bodysp();
         }
         return true;  // All is fine

test_regress/t/t_priority_case.out

@@ -0,0 +1,8 @@
+%Warning-CASEOVERLAP: t/t_priority_case.v:34:7: Case item ignored: every matching value is covered by an earlier item
+   34 |       2'b ?1: out1 = 3'd1;
+      |       ^~~~~~
+                      ... Use "/* verilator lint_off CASEOVERLAP */" and lint_on around source to disable this message.
+%Warning-CASEOVERLAP: t/t_priority_case.v:44:7: Case item ignored: every matching value is covered by an earlier item
+   44 |       2'b ?1: out1 = 3'd1;
+      |       ^~~~~~
+%Error: Exiting due to

test_regress/t/t_priority_case.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 2021 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},
+    );
+
+ok(1);
+1;

test_regress/t/t_priority_case.v

@@ -0,0 +1,50 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2021 by Rupert Swarbrick.
+// SPDX-License-Identifier: CC0-1.0
+
+module t(/*AUTOARG*/
+   // Inputs
+   value
+   );
+   input  [1:0] value;
+
+   sub u_sub(.value(value), .out0(), .out1());
+
+endmodule
+
+module sub (input logic [1:0]  value,
+            output logic [2:0] out0,
+            output logic [2:0] out1);
+
+  always_comb begin
+    // This case statement shouldn't cause any warnings. Although the cases overlap (2'b11 matches
+    // both 2'b?1 and 2'b1?), the second item matches 2'b10 and the first one doesn't.
+    priority casez (value)
+      2'b ?1: out0 = 3'd0;
+      2'b 1?: out0 = 3'd1;
+      default: out0 = 3'd2;
+    endcase
+
+    // This case statement *should* cause a warning: the second case is completely covered by the
+    // first.
+    priority casez (value)
+      2'b ?1: out1 = 3'd0;
+      2'b ?1: out1 = 3'd1;
+      default: out1 = 3'd2;
+    endcase
+
+    // This case statement should cause a warning too: the second case and third cases are
+    // completely covered by the first. But it should only cause one: like with overlapping cases,
+    // we assume that the author messed up the first case, so there's no real benefit to reporting
+    // each thing it subsumes.
+    priority casez (value)
+      2'b ?1: out1 = 3'd0;
+      2'b ?1: out1 = 3'd1;
+      2'b 11: out1 = 3'd2;
+      default: out1 = 3'd3;
+    endcase
+  end
+
+endmodule