adding support for configurable clock name matching via regex (#1205)

tools/tidy/README.md

@@ -55,15 +55,16 @@ Disable all: -*
 The `CheckConfigs` is a dictionary like, `config: value`, comma separated list of options for the different checks.
 The available options are:
 
-|            Config             |   Type   | Default |
-|:-----------------------------:|:--------:|:-------:|
-|          **clkName**          |  string  |  clk_i  |
-|         **resetName**         |  string  | rst_ni  |
-|     **resetIsActiveHigh**     |   bool   |  true   |
-|      **inputPortSuffix**      | [string] |  [_i]   |
-|     **outputPortSuffix**      | [string] |  [_o]   |
-|      **inoutPortSuffix**      | [string] |  [_io]  |
-| **moduleInstantiationPrefix** |  string  |   i_    |
+|            Config             |   Type   |      Default       |
+|:-----------------------------:|:--------:|:------------------:|
+|          **clkName**          |  string  |       clk_i        |
+|    **clkNameRegexString**     |  string  |  "clk\S*|clock\S*" |
+|         **resetName**         |  string  |      rst_ni        |
+|     **resetIsActiveHigh**     |   bool   |       true         |
+|      **inputPortSuffix**      | [string] |       [_i]         |
+|     **outputPortSuffix**      | [string] |       [_o]         |
+|      **inoutPortSuffix**      | [string] |       [_io]        |
+| **moduleInstantiationPrefix** |  string  |        i_          |
 
 An example of a possible configuration file:
 ```
@@ -74,6 +75,7 @@ Checks:
 
 CheckConfigs:
     clkName: clk,
+    clkNameRegexString: "clk_signal\S*|clock_port\S*",
     resetIsActiveHigh: false,
     inputPortSuffix: _k,
     outputPortSuffix: _p

tools/tidy/include/TidyConfig.h

@@ -10,6 +10,7 @@
 #include "TidyKind.h"
 #include <algorithm>
 #include <fmt/format.h>
+#include <regex>
 #include <slang/util/TypeTraits.h>
 #include <string>
 #include <unordered_map>
@@ -23,6 +24,8 @@ class TidyConfig {
     /// Configuration values of checks
     struct CheckConfigs {
         std::string clkName;
+        std::string clkNameRegexString;
+        std::regex clkNameRegexPattern;
         std::string resetName;
         bool resetIsActiveHigh;
         std::vector<std::string> inputPortSuffix;
@@ -94,6 +97,11 @@ class TidyConfig {
             visit(checkConfigs.resetName);
             return;
         }
+        else if (configName == "clkNameRegexString") {
+            visit(checkConfigs.clkNameRegexString);
+            checkConfigs.clkNameRegexPattern = std::regex(checkConfigs.clkNameRegexString);
+            return;
+        }
         else if (configName == "resetIsActiveHigh") {
             visit(checkConfigs.resetIsActiveHigh);
             return;

tools/tidy/src/TidyConfig.cpp

@@ -10,6 +10,8 @@ namespace fs = std::filesystem;
 
 TidyConfig::TidyConfig() {
     checkConfigs.clkName = "clk_i";
+    checkConfigs.clkNameRegexString = "clk\\S*|clock\\S*";
+    checkConfigs.clkNameRegexPattern = std::regex(checkConfigs.clkNameRegexString);
     checkConfigs.resetName = "rst_ni";
     checkConfigs.resetIsActiveHigh = true;
     checkConfigs.inputPortSuffix = {"_i"};

tools/tidy/src/TidyConfigParser.cpp

@@ -283,7 +283,16 @@ void TidyConfigParser::parseCheckConfigs() {
         // Parse multiple option values
         std::vector<std::string> optionValues;
 
+        auto isRegexMeta = [](char c) {
+            return c == '.' || c == '^' || c == '$' || c == '*' || c == '+' || c == '?' ||
+                   c == '{' || c == '}' || c == '[' || c == ']' || c == '\\' || c == '|' ||
+                   c == '(' || c == ')';
+        };
+
         auto isOptionValueChar = [](char c) { return isalnum(c) || c == '_'; };
+        auto isRegexOptionValueChar = [&](char c) {
+            return isalnum(c) || isRegexMeta(c) || c == '_';
+        };
 
         if (peekChar() == '[') {
             currentChar = nextChar(); // skip '['
@@ -301,6 +310,20 @@ void TidyConfigParser::parseCheckConfigs() {
             }
             currentChar = nextChar();
         }
+        else if (peekChar() == '"') {
+            currentChar = nextChar(); // skip '"'
+
+            std::string optionValue;
+            currentChar = readIf(optionValue, isRegexOptionValueChar);
+            if (!optionValue.empty())
+                optionValues.emplace_back((optionValue));
+
+            if (currentChar != '"') {
+                reportErrorAndExit(
+                    fmt::format("Expected '\"' but found ({}){}", +currentChar, currentChar));
+            }
+            currentChar = nextChar();
+        }
         else { // Parse single option value
             std::string optionValue;
             currentChar = readIf(optionValue, isOptionValueChar);

tools/tidy/src/synthesis/UnusedSensitiveSignal.cpp

@@ -5,6 +5,7 @@
 #include "TidyDiags.h"
 #include "fmt/color.h"
 #include <algorithm>
+#include <regex>
 
 #include "slang/syntax/AllSyntax.h"
 
@@ -54,7 +55,10 @@ struct MainVisitor : public TidyVisitor, ASTVisitor<MainVisitor, true, true> {
                             std::back_inserter(unusedSignals), compare);
 
         for (auto signal : unusedSignals) {
-            if (signal.first != config.getCheckConfigs().clkName)
+            // either match against clkName or against the regex pattern
+            if (signal.first != config.getCheckConfigs().clkName &&
+                !(std::regex_match(std::string(signal.first),
+                                   config.getCheckConfigs().clkNameRegexPattern)))
                 diags.add(diag::UnusedSensitiveSignal, signal.second) << signal.first;
         }
     }

tools/tidy/tests/TidyConfigParserTest.cpp

@@ -103,6 +103,7 @@ TEST_CASE("TidyParser: Disable some checks") {
 TEST_CASE("TidyParser: Set check config") {
     auto config_str = std::string(R"(CheckConfigs:
     clkName: clk,
+    clkNameRegexString: "clock\S.*",
     resetIsActiveHigh: false,
     inputPortSuffix: _k,
     inputPortSuffix: _p)");
@@ -112,6 +113,7 @@ TEST_CASE("TidyParser: Set check config") {
 
     CHECK(config.getCheckConfigs().clkName == "clk");
     CHECK(config.getCheckConfigs().resetName == "rst_ni");
+    CHECK(config.getCheckConfigs().clkNameRegexString == "clock\\S.*");
     CHECK_FALSE(config.getCheckConfigs().resetIsActiveHigh);
     CHECK(config.getCheckConfigs().inputPortSuffix == std::vector<std::string>{"_p"});
 }

tools/tidy/tests/UnusedSensitiveSignalTest.cpp

@@ -114,3 +114,78 @@ endmodule
     bool result = visitor->check(root);
     CHECK(result);
 }
+
+TEST_CASE(
+    "UnusedSensitiveSignal: Clock signals not matching regex expression will raise a warning") {
+    auto tree = SyntaxTree::fromText(R"(
+module d_ff_a_en
+(
+    input logic ck_a_i, rst_i, en_i, c_i, d_i,
+
+    output logic q_o
+) ;
+always_ff @ (posedge ck_a_i, posedge rst_i) begin
+    if (rst_i)
+        q_o <= '0;
+    else if (en_i)
+        q_o <= d_i;
+    end
+endmodule
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+    compilation.getAllDiagnostics();
+    auto& root = compilation.getRoot();
+
+    TidyConfig config;
+    Registry::setConfig(config);
+    Registry::setSourceManager(compilation.getSourceManager());
+    auto visitor = Registry::create("UnusedSensitiveSignal");
+    bool result = visitor->check(root);
+    CHECK_FALSE(result);
+}
+
+TEST_CASE("UnusedSensitiveSignal: Clock signals matching regex expression won't raise a warning") {
+    auto tree = SyntaxTree::fromText(R"(
+module d_ff_a_en
+(
+    input logic clock_a_i, rst_i, en_i, c_i, d_i,
+
+    output logic q_o
+) ;
+always_ff @ (posedge clock_a_i, posedge rst_i) begin
+    if (rst_i)
+        q_o <= '0;
+    else if (en_i)
+        q_o <= d_i;
+    end
+endmodule
+
+module d_ff_b_en
+(
+    input logic clk_b_i, rst_i, en_i, c_i, d_i,
+
+    output logic q_o
+) ;
+always_ff @ (posedge clk_b_i, posedge rst_i) begin
+    if (rst_i)
+        q_o <= '0;
+    else if (en_i)
+        q_o <= d_i;
+    end
+endmodule
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+    compilation.getAllDiagnostics();
+    auto& root = compilation.getRoot();
+
+    TidyConfig config;
+    Registry::setConfig(config);
+    Registry::setSourceManager(compilation.getSourceManager());
+    auto visitor = Registry::create("UnusedSensitiveSignal");
+    bool result = visitor->check(root);
+    CHECK(result);
+}