Don't error if multiple UDP row inputs match, unless their output dif…

…fer (#931)
MikePopoloski · Apr 4, 2024 · adf62ec · adf62ec
1 parent 9e15d4e
commit adf62ec
Show file tree

Hide file tree

Showing 2 changed files with 125 additions and 8 deletions.
diff --git a/source/ast/symbols/MemberSymbols.cpp b/source/ast/symbols/MemberSymbols.cpp
@@ -995,10 +995,9 @@ static void createTableRow(const Scope& scope, const UdpEntrySyntax& syntax,
         return;
     }
 
-    char stateChar = 0;
-    if (syntax.current) {
-        if (syntax.current->kind == SyntaxKind::UdpSimpleField) {
-            auto raw = syntax.current->as<UdpSimpleFieldSyntax>().field.rawText();
+    auto getStateChar = [](const UdpFieldBaseSyntax* base) -> char {
+        if (base && base->kind == SyntaxKind::UdpSimpleField) {
+            auto raw = base->as<UdpSimpleFieldSyntax>().field.rawText();
             if (raw.size() == 1) {
                 auto c = charToLower(raw[0]);
                 switch (c) {
@@ -1007,14 +1006,18 @@ static void createTableRow(const Scope& scope, const UdpEntrySyntax& syntax,
                     case 'x':
                     case '?':
                     case 'b':
-                        stateChar = c;
-                        break;
+                        return c;
                     default:
                         break;
                 }
             }
         }
+        return 0;
+    };
 
+    char stateChar = 0;
+    if (syntax.current) {
+        stateChar = getStateChar(syntax.current);
         if (!stateChar)
             return;
     }
@@ -1042,12 +1045,32 @@ static void createTableRow(const Scope& scope, const UdpEntrySyntax& syntax,
     if (!outputChar)
         return;
 
+    auto matchOutput = [](char state1, char output1, char output2) -> bool {
+        if (output1 != '-')
+            return false;
+        switch (state1) {
+            case '0':
+            case '1':
+            case 'x':
+                return (output2 == state1);
+            case 'b':
+                return (output2 == '0' || output2 == '1');
+            case '?':
+                return (output2 == '0' || output2 == '1' || output2 == 'x');
+            default:
+                return false; // should never happen
+        }
+    };
+
     auto existing = trie.insert(syntax, inputs, stateChar, trieAlloc);
     if (existing) {
         // This is an error if the existing row has a different output,
         // otherwise it's just silently ignored.
         auto existingOutput = getOutputChar(existing->next);
-        if (existingOutput != outputChar) {
+        auto existingState = getStateChar(existing->current);
+        if (!((existingOutput == outputChar) ||
+              matchOutput(existingState, existingOutput, outputChar) ||
+              matchOutput(stateChar, outputChar, existingOutput))) {
             auto& diag = scope.addDiag(diag::UdpDupDiffOutput, syntax.sourceRange());
             diag.addNote(diag::NotePreviousDefinition, existing->sourceRange());
             return;

diff --git a/tests/unittests/ast/PrimitiveTests.cpp b/tests/unittests/ast/PrimitiveTests.cpp
@@ -176,13 +176,22 @@ primitive p14 (a, b, c);
         xx:0:0;
     endtable
 endprimitive
+
+primitive p15 (a, b);
+    output a;
+    input b;
+    table
+        0:0;
+        0:1;
+    endtable
+endprimitive
 )");
 
     Compilation compilation;
     compilation.addSyntaxTree(tree);
 
     auto& diags = compilation.getAllDiagnostics();
-    REQUIRE(diags.size() == 24);
+    REQUIRE(diags.size() == 25);
     CHECK(diags[0].code == diag::PrimitiveOutputFirst);
     CHECK(diags[1].code == diag::PrimitiveAnsiMix);
     CHECK(diags[2].code == diag::DuplicateDefinition);
@@ -207,6 +216,7 @@ endprimitive
     CHECK(diags[21].code == diag::UdpWrongInputCount);
     CHECK(diags[22].code == diag::UdpDupDiffOutput);
     CHECK(diags[23].code == diag::UdpAllX);
+    CHECK(diags[24].code == diag::UdpDupDiffOutput);
 }
 
 TEST_CASE("UDP instances error checking") {
@@ -438,6 +448,90 @@ endmodule
     CHECK(diags[2].code == diag::UdpDupDiffOutput);
 }
 
+TEST_CASE("UDP overlapping inputs with compatible outputs") {
+    auto tree = SyntaxTree::fromText(R"(
+ primitive X1 (q, clk, d);
+    output reg q;
+    input clk, d;
+    table
+    // clk in  : Qt  : Qt+1
+    r    0   : ?   :  0;
+    *    0   : 0   :  -;
+    endtable
+ endprimitive
+ primitive X2 (q, clk, d);
+    output reg q;
+    input clk, d;
+    table
+    // clk in  : Qt  : Qt+1
+    r    0   : 1   :  -;
+    *    0   : ?   :  1;
+    endtable
+ endprimitive
+ primitive X3 (q, clk, d);
+    output reg q;
+    input clk, d;
+    table
+    // clk in  : Qt  : Qt+1
+    r    0   : ?   :  -;
+    *    0   : ?   :  0;
+    *    1   : ?   :  0;
+    r    1   : ?   :  -;
+    endtable
+ endprimitive
+ primitive X4 (q, clk, d);
+    output reg q;
+    input clk, d;
+    table
+    // clk in  : Qt  : Qt+1
+    r    0   : ?   :  -;
+    *    0   : ?   :  1;
+    *    1   : ?   :  1;
+    r    1   : ?   :  -;
+    endtable
+ endprimitive
+ primitive X5 (q, clk, d);
+    output reg q;
+    input clk, d;
+    table
+    // clk in  : Qt  : Qt+1
+    r    0   : ?   :  -;
+    *    0   : ?   :  x;
+    *    1   : ?   :  x;
+    r    1   : ?   :  -;
+    endtable
+ endprimitive
+ primitive X6 (q, clk, d);
+    output reg q;
+    input clk, d;
+    table
+    // clk in  : Qt  : Qt+1
+    r    0   : b   :  -;
+    *    0   : ?   :  0;
+    *    1   : ?   :  0;
+    r    1   : b   :  -;
+    endtable
+ endprimitive
+primitive X7 (q, clk, d);
+    output reg q;
+    input clk, d;
+    table
+    // clk in  : Qt  : Qt+1
+    r    0   : b   :  -;
+    *    0   : ?   :  1;
+    *    1   : ?   :  1;
+    r    1   : b   :  -;
+    endtable
+ endprimitive
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+
+    auto& diags = compilation.getAllDiagnostics();
+    REQUIRE(diags.size() == 0);
+}
+
 TEST_CASE("More UDP error cases") {
     auto tree = SyntaxTree::fromText(R"(
 primitive p1(output reg o, input a);