diff --git a/include/slang/ast/symbols/MemberSymbols.h b/include/slang/ast/symbols/MemberSymbols.h index b239c6a6e..64dfd6e5e 100644 --- a/include/slang/ast/symbols/MemberSymbols.h +++ b/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 ports; std::span 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, diff --git a/scripts/diagnostics.txt b/scripts/diagnostics.txt index 5433f4e43..40b7f02c8 100644 --- a/scripts/diagnostics.txt +++ b/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 diff --git a/scripts/warning_docs.txt b/scripts/warning_docs.txt index 9841d35b1..381b6fb27 100644 --- a/scripts/warning_docs.txt +++ b/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 +``` diff --git a/source/ast/symbols/MemberSymbols.cpp b/source/ast/symbols/MemberSymbols.cpp index 5883a6398..a3b5d86d1 100644 --- a/source/ast/symbols/MemberSymbols.cpp +++ b/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 - std::optional> insert(const UdpEntrySyntax& syntax, - std::span inputs, - char stateChar, - TAllocator& allocator) { + void find(BitTrie*& primaryNode, std::span inputs, char stateChar, + TAllocator& allocator, SmallVector& result) { SmallVector nodes; - BitTrie* primary = this; nodes.push_back(this); auto advance = [&](char c) { SmallVector 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 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 + std::optional> insert(const UdpEntrySyntax& syntax, + std::span inputs, + char stateChar, + TAllocator& allocator) { + BitTrie* primary = this; + SmallVector 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 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(); inputs.push_back('('); @@ -989,6 +996,18 @@ static void createTableRow(const Scope& scope, const UdpEntrySyntax& syntax, auto tok = input->as().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 trieAlloc(alloc); SmallVector 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 edgeScope = {"(01)", "(0x)", "(10)", + "(1x)", "(x0)", "(x1)"}; + // Init state is (??)000... + std::string_view initEdge = "(??" + ")"; + std::vector initialState(initEdge.begin(), initEdge.end()); + initialState.insert(initialState.end(), numInput - 1, '0'); + std::vector state = initialState; + // End state is xxx...(x1) + std::string_view endEdge = "(x1)"; + std::vector 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& 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 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 = ≜ + SmallVector 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); diff --git a/tests/unittests/ast/PrimitiveTests.cpp b/tests/unittests/ast/PrimitiveTests.cpp index 68ca06006..6136e593e 100644 --- a/tests/unittests/ast/PrimitiveTests.cpp +++ b/tests/unittests/ast/PrimitiveTests.cpp @@ -16,6 +16,10 @@ primitive srff (q, s, r); 0 r : ? : 0 ; 0 f : 0 : - ; 1 1 : ? : 0 ; + p ? : ? : 0 ; + n ? : ? : - ; + ? n : ? : 0 ; + ? p : ? : 0 ; endtable endprimitive : srff @@ -191,7 +195,7 @@ endprimitive compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 25); + REQUIRE(diags.size() == 26); CHECK(diags[0].code == diag::PrimitiveOutputFirst); CHECK(diags[1].code == diag::PrimitiveAnsiMix); CHECK(diags[2].code == diag::DuplicateDefinition); @@ -214,9 +218,10 @@ endprimitive CHECK(diags[19].code == diag::ExpectedUdpSymbol); CHECK(diags[20].code == diag::UdpWrongInputCount); CHECK(diags[21].code == diag::UdpWrongInputCount); - CHECK(diags[22].code == diag::UdpDupDiffOutput); - CHECK(diags[23].code == diag::UdpAllX); - CHECK(diags[24].code == diag::UdpDupDiffOutput); + CHECK(diags[22].code == diag::UdpCoverage); + CHECK(diags[23].code == diag::UdpDupDiffOutput); + CHECK(diags[24].code == diag::UdpAllX); + CHECK(diags[25].code == diag::UdpDupDiffOutput); } TEST_CASE("UDP instances error checking") { @@ -320,19 +325,20 @@ endprimitive compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 12); - CHECK(diags[0].code == diag::UdpInvalidSymbol); - CHECK(diags[1].code == diag::UdpInvalidTransition); - CHECK(diags[2].code == diag::UdpInvalidEdgeSymbol); - CHECK(diags[3].code == diag::UdpTransitionLength); - CHECK(diags[4].code == diag::UdpSingleChar); - CHECK(diags[5].code == diag::UdpInvalidInputOnly); - CHECK(diags[6].code == diag::UdpInvalidMinus); + REQUIRE(diags.size() == 13); + CHECK(diags[0].code == diag::UdpCoverage); + CHECK(diags[1].code == diag::UdpInvalidSymbol); + CHECK(diags[2].code == diag::UdpInvalidTransition); + CHECK(diags[3].code == diag::UdpInvalidEdgeSymbol); + CHECK(diags[4].code == diag::UdpTransitionLength); + CHECK(diags[5].code == diag::UdpSingleChar); + CHECK(diags[6].code == diag::UdpInvalidInputOnly); CHECK(diags[7].code == diag::UdpInvalidMinus); - CHECK(diags[8].code == diag::UdpInvalidOutput); - CHECK(diags[9].code == diag::UdpDupTransition); - CHECK(diags[10].code == diag::UdpSequentialState); - CHECK(diags[11].code == diag::UdpCombState); + CHECK(diags[8].code == diag::UdpInvalidMinus); + CHECK(diags[9].code == diag::UdpInvalidOutput); + CHECK(diags[10].code == diag::UdpDupTransition); + CHECK(diags[11].code == diag::UdpSequentialState); + CHECK(diags[12].code == diag::UdpCombState); } TEST_CASE("UDP row error checking regress") { @@ -390,6 +396,7 @@ primitive edet (q, i); table (?x) : ? : 1; (x?) : ? : 0; + (??) : ? : -; endtable endprimitive )"); @@ -428,8 +435,9 @@ primitive p(output reg o, input a, b); (x1)1:1:0; p1:1:x; (x0)1:1:1; - *1:1:0; + *?:1:0; n1:1:0; + ?*:1:0; endtable endprimitive @@ -488,6 +496,8 @@ TEST_CASE("UDP overlapping inputs with compatible outputs") { // clk in : Qt : Qt+1 r 0 : ? : 0; * 0 : 0 : -; + * ? : b : -; + ? * : b : -; endtable endprimitive primitive X2 (q, clk, d); @@ -497,6 +507,8 @@ TEST_CASE("UDP overlapping inputs with compatible outputs") { // clk in : Qt : Qt+1 r 0 : 1 : -; * 0 : ? : 1; + * ? : b : -; + ? * : b : -; endtable endprimitive primitive X3 (q, clk, d); @@ -508,6 +520,8 @@ TEST_CASE("UDP overlapping inputs with compatible outputs") { * 0 : ? : 0; * 1 : ? : 0; r 1 : ? : -; + * ? : b : -; + ? * : b : -; endtable endprimitive primitive X4 (q, clk, d); @@ -519,6 +533,8 @@ TEST_CASE("UDP overlapping inputs with compatible outputs") { * 0 : ? : 1; * 1 : ? : 1; r 1 : ? : -; + * ? : b : -; + ? * : b : -; endtable endprimitive primitive X5 (q, clk, d); @@ -530,6 +546,8 @@ TEST_CASE("UDP overlapping inputs with compatible outputs") { * 0 : ? : x; * 1 : ? : x; r 1 : ? : -; + * ? : b : x; + ? * : b : -; endtable endprimitive primitive X6 (q, clk, d); @@ -541,6 +559,8 @@ TEST_CASE("UDP overlapping inputs with compatible outputs") { * 0 : ? : 0; * 1 : ? : 0; r 1 : b : -; + * ? : b : -; + ? * : b : -; endtable endprimitive primitive X7 (q, clk, d); @@ -552,6 +572,8 @@ primitive X7 (q, clk, d); * 0 : ? : 1; * 1 : ? : 1; r 1 : b : -; + * ? : b : -; + ? * : b : -; endtable endprimitive )"); @@ -583,10 +605,12 @@ endprimitive compilation.addSyntaxTree(tree); auto& diags = compilation.getAllDiagnostics(); - REQUIRE(diags.size() == 3); - CHECK(diags[0].code == diag::UdpTransSameChar); - CHECK(diags[1].code == diag::UdpEdgeInComb); - CHECK(diags[2].code == diag::UdpInvalidMinus); + REQUIRE(diags.size() == 5); + CHECK(diags[0].code == diag::UdpCoverage); + CHECK(diags[1].code == diag::UdpTransSameChar); + CHECK(diags[2].code == diag::UdpCoverage); + CHECK(diags[3].code == diag::UdpEdgeInComb); + CHECK(diags[4].code == diag::UdpInvalidMinus); } TEST_CASE("Most gates can't attach to user-defined nettypes") { @@ -636,3 +660,85 @@ endmodule CHECK(diags[4].code == diag::BiDiSwitchNetTypes); CHECK(diags[5].code == diag::BiDiSwitchNetTypes); } + +TEST_CASE("UDP edge-sequence coverage no-error tests") { + auto tree = SyntaxTree::fromText(R"( +primitive p1 (q, clock, data); + output q; reg q; + input clock, data; + table + // clock data q q+ + (01) ? : ? : 0 ; + (??) ? : ? : 0 ; + ? (??) : ? : 0 ; + endtable +endprimitive +primitive p2 (q, clock, data, data1); +output q; reg q; + input clock, data, data1; + table + // clock data q q+ + n ? 1 : ? : 0 ; + p ? 1 : ? : 0 ; + n ? ? : ? : 0 ; + p ? ? : ? : 0 ; + ? (??) ? : ? : 0 ; + ? ? (??) : ? : 0 ; + endtable +endprimitive +primitive p3 (q, clock, data); + output q; reg q; + input clock, data; + table + // clock data q q+ + (??) ? : ? : 0 ; + ? (??) : ? : 0 ; + endtable +endprimitive +)"); + + Compilation compilation; + compilation.addSyntaxTree(tree); + + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 0); +} + +TEST_CASE("UDP edge-sequence coverage error tests") { + auto tree = SyntaxTree::fromText(R"( +primitive p1 (q, clock, data); + output q; reg q; + input clock, data; + table + // clock data q q+ + (01) ? : ? : 0 ; + endtable +endprimitive +primitive p2 (q, clock, data, data1); + output q; reg q; + input clock, data, data1; + table + // clock data q q+ + n ? 1 : ? : 0 ; + p ? 1 : ? : 0 ; + endtable +endprimitive +primitive p3 (q, clock, data); + output q; reg q; + input clock, data; + table + // clock data q q+ + (??) ? : ? : 0 ; + endtable +endprimitive +)"); + + Compilation compilation; + compilation.addSyntaxTree(tree); + + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 3); + CHECK(diags[0].code == diag::UdpCoverage); + CHECK(diags[1].code == diag::UdpCoverage); + CHECK(diags[2].code == diag::UdpCoverage); +}