[slang] Add an inputs/edges combination coverage check for udp (#1087)

include/slang/ast/symbols/MemberSymbols.h

@@ -261,12 +261,14 @@ class SLANG_EXPORT PrimitiveSymbol : public Symbol, public Scope {
         std::string_view inputs;
         char state = 0;
         char output = 0;
+        bool isEdgeSensitive = false;
     };
 
     std::span<const PrimitivePortSymbol* const> ports;
     std::span<const TableEntry> table;
     const ConstantValue* initVal = nullptr;
     bool isSequential = false;
+    bool isEdgeSensitive = false;
     enum PrimitiveKind { UserDefined, Fixed, NInput, NOutput, BiDiSwitch } primitiveKind;
 
     PrimitiveSymbol(Compilation& compilation, std::string_view name, SourceLocation loc,

scripts/diagnostics.txt

@@ -542,6 +542,8 @@ warning warning-task WarningTask "$warning encountered{}"
 note InfoTask "$info encountered{}"
 note NoteComparisonReduces "comparison reduces to ({} {} {})"
 note NoteCommonAncestor "common ancestor here violates that constraint"
+warning udp-coverage UdpCoverage "if the behavior of the UDP is sensitive to edges of any input, the desired output state shall be specified for all edges of all inputs."
+note NoteUdpCoverage "missed desired output for rows:\n{}"
 warning explicit-static StaticInitializerMustBeExplicit "initializing a static variable in a procedural context requires an explicit 'static' keyword"
 warning case-gen-dup CaseGenerateDup "more than one case generate block matches the value {}"
 warning case-gen-none CaseGenerateNoBlock "no case generate expression matches the value {}"
@@ -1147,7 +1149,7 @@ group extra = { empty-member empty-stmt dup-import pointless-void-cast case-gen-
                 float-narrow }
 
 group pedantic = { empty-pattern implicit-net-port nonstandard-escape-code nonstandard-generate format-multibit-strength
-                   nonstandard-sys-func nonstandard-foreach nonstandard-dist isunbounded-param-arg }
+                   nonstandard-sys-func nonstandard-foreach nonstandard-dist isunbounded-param-arg udp-coverage }
 
 group conversion = { width-trunc width-expand port-width-trunc port-width-expand implicit-conv
                      constant-conversion sign-conversion float-bool-conv int-bool-conv float-int-conv

scripts/warning_docs.txt

@@ -1353,3 +1353,29 @@ module m;
     end
 endmodule
 ```
+
+-Wudp-coverage
+Checks if the behavior of the UDP is sensitive to edges of any input,
+the desired output state shall be specified for all edges of all inputs.
+That constraint is specified by LRM 29.6 section and it is not clear about it.
+This can be a useful for checking edge-sensitive UDP missed transitions
+when developing and simulating a behavioral model of industrial cells
+which commonly implemented as verilog primitives.
+```
+primitive d_edge_ff (q, clock, data);
+    output q; reg q;
+    input clock, data;
+    table
+        // clock data q q+
+        // obtain output on rising edge of clock
+        (01) 0 : ? : 0 ;
+        (01) 1 : ? : 1 ;
+        (0?) 1 : 1 : 1 ;
+        (0?) 0 : 0 : 0 ;
+        // ignore negative edge of clock
+        (?0) ? : ? : - ;
+        // missed clock edge (1x) specification
+        // missed edge specifications for data signal
+    endtable
+endprimitive
+```

source/ast/symbols/MemberSymbols.cpp

@@ -766,23 +766,16 @@ void PrimitivePortSymbol::serializeTo(ASTSerializer& serializer) const {
 // Each 'bit' is one input value in the row.
 class BitTrie {
 public:
-    // Tries to insert the row. Returns nullopt if the row is
-    // successfully inserted, and a set of the previously
-    // inserted rows with the same bit pattern if a collision
-    // is found.
     template<typename TAllocator>
-    std::optional<SmallVector<const UdpEntrySyntax*, 4>> insert(const UdpEntrySyntax& syntax,
-                                                                std::span<const char> inputs,
-                                                                char stateChar,
-                                                                TAllocator& allocator) {
+    void find(BitTrie*& primaryNode, std::span<const char> inputs, char stateChar,
+              TAllocator& allocator, SmallVector<const UdpEntrySyntax*, 4>& result) {
         SmallVector<BitTrie*> nodes;
-        BitTrie* primary = this;
         nodes.push_back(this);
 
         auto advance = [&](char c) {
             SmallVector<BitTrie*> nextNodes;
             for (auto node : nodes)
-                node->nextNodesFor(c, nextNodes, primary, allocator);
+                node->nextNodesFor(c, nextNodes, primaryNode, allocator);
             nodes = std::move(nextNodes);
         };
 
@@ -828,12 +821,24 @@ class BitTrie {
 
         // If any of our nodes have entries we won't insert,
         // as it means we have a duplicate.
-        SmallVector<const UdpEntrySyntax*, 4> eSyntaxes;
         for (auto node : nodes) {
             if (node->entry)
-                eSyntaxes.push_back(node->entry);
+                result.push_back(node->entry);
         }
+    }
 
+    // Tries to insert the row. Returns nullopt if the row is
+    // successfully inserted, and a set of the previously
+    // inserted rows with the same bit pattern if a collision
+    // is found.
+    template<typename TAllocator>
+    std::optional<SmallVector<const UdpEntrySyntax*, 4>> insert(const UdpEntrySyntax& syntax,
+                                                                std::span<const char> inputs,
+                                                                char stateChar,
+                                                                TAllocator& allocator) {
+        BitTrie* primary = this;
+        SmallVector<const UdpEntrySyntax*, 4> eSyntaxes;
+        find(primary, inputs, stateChar, allocator, eSyntaxes);
         // Always store so as not to miss info it in case of possible overlap.
         // If the primary->entry already has a value,
         // then rewriting will not spoil anything,
@@ -948,8 +953,10 @@ static void createTableRow(const Scope& scope, const UdpEntrySyntax& syntax,
     SmallVector<char> inputs;
     size_t numInputs = 0;
     bool allX = true;
+    bool isEdgeSensitive = false;
     for (auto input : syntax.inputs) {
         if (input->kind == SyntaxKind::UdpEdgeField) {
+            isEdgeSensitive = true;
             auto& edge = input->as<UdpEdgeFieldSyntax>();
             inputs.push_back('(');
 
@@ -989,6 +996,18 @@ static void createTableRow(const Scope& scope, const UdpEntrySyntax& syntax,
             auto tok = input->as<UdpSimpleFieldSyntax>().field;
             for (auto c : tok.rawText()) {
                 auto d = charToLower(c);
+                switch (d) {
+                    case '*':
+                    case 'r':
+                    case 'f':
+                    case 'p':
+                    case 'n':
+                        isEdgeSensitive = true;
+                        break;
+                    default:
+                        break;
+                }
+
                 inputs.push_back(d);
                 numInputs++;
                 allX &= d == 'x';
@@ -1094,7 +1113,8 @@ static void createTableRow(const Scope& scope, const UdpEntrySyntax& syntax,
     }
 
     auto inputSpan = inputs.copy(scope.getCompilation());
-    table.push_back({std::string_view(inputSpan.data(), inputSpan.size()), stateChar, outputChar});
+    table.push_back({std::string_view(inputSpan.data(), inputSpan.size()), stateChar, outputChar,
+                     isEdgeSensitive});
 }
 
 PrimitiveSymbol& PrimitiveSymbol::fromSyntax(const Scope& scope,
@@ -1337,10 +1357,174 @@ PrimitiveSymbol& PrimitiveSymbol::fromSyntax(const Scope& scope,
         BumpAllocator alloc;
         PoolAllocator<BitTrie> trieAlloc(alloc);
         SmallVector<TableEntry> table;
-        for (auto entry : syntax.body->entries)
+        for (auto entry : syntax.body->entries) {
             createTableRow(scope, *entry, table, ports.size(), trie, trieAlloc);
+            if (!table.empty() && !prim->isEdgeSensitive)
+                prim->isEdgeSensitive = table.back().isEdgeSensitive;
+        }
 
         prim->table = table.copy(comp);
+
+        // Check that if the behavior of the UDP is sensitive to edges of any input, the desired
+        // output state shall be specified for all edges of all inputs.
+        if (prim->isEdgeSensitive && !prim->table.empty()) {
+            auto portSize = ports.size();
+            auto numInput = (portSize >= 2) ? portSize - 1 : portSize;
+            // Ordered list of all possible clock edges
+            const static std::vector<std::string_view> edgeScope = {"(01)", "(0x)", "(10)",
+                                                                    "(1x)", "(x0)", "(x1)"};
+            // Init state is (??)000...
+            std::string_view initEdge = "(??"
+                                        ")";
+            std::vector<char> initialState(initEdge.begin(), initEdge.end());
+            initialState.insert(initialState.end(), numInput - 1, '0');
+            std::vector<char> state = initialState;
+            // End state is xxx...(x1)
+            std::string_view endEdge = "(x1)";
+            std::vector<char> endState(numInput - 1, 'x');
+            endState.insert(endState.end(), endEdge.begin(), endEdge.end());
+            // Iterate state by increase leftoutermost inputs.
+            // For simple inputs value '0' goes to '1' and value '1' goes to `x`.
+            // For edge inputs circular iteration occurs over 'edgeScope' list.
+            auto incrementState = [&initEdge](std::vector<char>& state) {
+                for (unsigned i = 0; i < state.size(); ++i) {
+                    // Increase edge input
+                    if (state.at(i) == '(') {
+                        std::string currEdge{state.at(i), state.at(i + 1), state.at(i + 2),
+                                             state.at(i + 3)};
+                        if (currEdge == edgeScope.back()) {
+                            if (state.back() != 'x') {
+                                if (i != 0) {
+                                    std::copy(edgeScope[0].begin(), edgeScope[0].end(),
+                                              state.begin() + i);
+                                }
+                                else {
+                                    // If it is edge input at first position set it as '(??)'
+                                    // instead of '(01)' for heuristically check it and skip any
+                                    // other edge inputs if '(??)' is present.
+                                    std::copy(initEdge.begin(), initEdge.end(), state.begin());
+                                }
+
+                                i += 3;
+                                continue;
+                            }
+                            else {
+                                // Move edge input to the next position
+                                // and set first input as '?' and all other as '0'
+                                state.at(0) = '?';
+                                for (unsigned j = 1; j < state.size(); ++j)
+                                    state.at(j) = '0';
+
+                                std::copy(edgeScope[0].begin(), edgeScope[0].end(),
+                                          state.begin() + i + 1);
+                                break;
+                            }
+                        }
+                        else {
+                            size_t nextPos =
+                                (state.at(i + 1) != '?')
+                                    ? (size_t)std::distance(edgeScope.begin(),
+                                                            std::find(edgeScope.begin(),
+                                                                      edgeScope.end(), currEdge)) +
+                                          1
+                                    : 0U;
+                            std::copy(edgeScope[nextPos].begin(), edgeScope[nextPos].end(),
+                                      state.begin() + i);
+                            break;
+                        }
+                    }
+
+                    if (state.at(i) == '0') {
+                        state.at(i) = '1';
+                        break;
+                    }
+
+                    if (state.at(i) == '1') {
+                        state.at(i) = 'x';
+                        break;
+                    }
+
+                    if (i == 0) {
+                        // If it is simple input at first position set it as '?'
+                        // instead of '0' for heuristically check it and skip any
+                        // other inputs if '?' is present.
+
+                        if (state.at(i) == '?') {
+                            state.at(i) = '0';
+                            break;
+                        }
+
+                        if (state.at(i) == 'x')
+                            state.at(i) = '?';
+                        continue;
+                    }
+
+                    state.at(i) = '0';
+                }
+
+                return state.at(0) == '?' || state.at(1) == '?';
+            };
+
+            Diagnostic* diag = nullptr;
+            SmallVector<std::string, 8> missingRows;
+            bool isVariable = true;
+            // Try to find the missing combinations by completely enumerating all possible table
+            // rows
+            bool next = true;
+            while (next) {
+                // Comparing current state with end state
+                if (state.back() == ')' && (numInput == 1 || state[0] == 'x'))
+                    next = (state != endState);
+
+                BitTrie* triePtr = &trie;
+                SmallVector<const UdpEntrySyntax*, 4> results;
+                // Try to find a desired input in an existing table structure
+                trie.find(triePtr, state, '?', trieAlloc, results);
+                if (results.empty()) {
+                    if (!diag)
+                        diag = &scope.addDiag(diag::UdpCoverage, prim->location);
+
+                    std::string noteStr;
+                    bool nextSplit = true;
+                    for (auto c : state) {
+                        if (c == '(')
+                            nextSplit = false;
+                        else if (c == ')')
+                            nextSplit = true;
+
+                        noteStr += c;
+                        if (nextSplit)
+                            noteStr += ' ';
+                    }
+                    missingRows.push_back(noteStr);
+
+                    bool wasVariable = isVariable;
+                    // If state with first any value input ('(??)' for edge input or '?' for simple
+                    // input) was not found then we don't need to check any other state with
+                    // explicit clock edges. So skip the state until we get a new any value clock
+                    // edge state.
+                    do {
+                        isVariable = incrementState(state);
+                        if (state.back() == ')' && (numInput == 1 || state[0] == 'x'))
+                            next = (state != endState);
+                    } while (next && wasVariable && !isVariable);
+
+                    continue;
+                }
+
+                isVariable = incrementState(state);
+            }
+
+            if (diag && !missingRows.empty()) {
+                std::string noteStr;
+                for (auto& missingRow : missingRows) {
+                    noteStr += {missingRow.data(), missingRow.size()};
+                    noteStr += "\n";
+                }
+
+                diag->addNote(diag::NoteUdpCoverage, prim->location) << noteStr;
+            }
+        }
     }
 
     prim->ports = ports.copy(comp);