From f63ac1fc562ae9cb0f50a02467884af0016805e2 Mon Sep 17 00:00:00 2001 From: karimtera Date: Sun, 10 Jul 2022 02:14:44 +0200 Subject: [PATCH 01/17] verible standalone preprocessor: multiple-cu mode [WIP] --- verilog/tools/preprocessor/BUILD | 2 + .../preprocessor/verilog_preprocessor.cc | 51 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/verilog/tools/preprocessor/BUILD b/verilog/tools/preprocessor/BUILD index 4216fe9e8..f41b865d7 100644 --- a/verilog/tools/preprocessor/BUILD +++ b/verilog/tools/preprocessor/BUILD @@ -14,6 +14,8 @@ cc_binary( "//common/util:init_command_line", "//common/util:subcommand", "//verilog/transform:strip_comments", + "//verilog/preprocessor:verilog_preprocess", + "//verilog/parser:verilog_lexer", "@com_google_absl//absl/flags:usage", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 6e0121e83..8aa0dd2e4 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -24,6 +24,8 @@ #include "common/util/init_command_line.h" #include "common/util/subcommand.h" #include "verilog/transform/strip_comments.h" +#include "verilog/parser/verilog_lexer.h" +#include "verilog/preprocessor/verilog_preprocess.h" using verible::SubcommandArgsRange; using verible::SubcommandEntry; @@ -63,6 +65,44 @@ static absl::Status StripComments(const SubcommandArgsRange& args, return absl::OkStatus(); } +static absl::Status MultipleCU(const SubcommandArgsRange& args, + std::istream&, std::ostream& outs, + std::ostream&) { + if (args.empty()) { + return absl::InvalidArgumentError( + "Missing file argument."); + } + + for(auto source_file:args){ + std::string source_contents; + if (auto status = verible::file::GetContents(source_file, &source_contents); + !status.ok()) { + return status; + } + verilog::VerilogPreprocess::Config config; + config.filter_branches=1; + verilog::VerilogPreprocess preprocessor(config); + verilog::VerilogLexer lexer(source_contents); + verible::TokenSequence lexed_sequence; + for (lexer.DoNextToken(); !lexer.GetLastToken().isEOF(); + lexer.DoNextToken()) { + // For now we will store the syntax tree tokens only, ignoring all the white-space characters. + // however that should be stored to output the source code just like it was, but with conditionals filtered. + if(lexer.KeepSyntaxTreeTokens(lexer.GetLastToken())) lexed_sequence.push_back(lexer.GetLastToken()); + } + verible::TokenStreamView lexed_streamview; + // Initializing the lexed token stream view. + InitTokenStreamView(lexed_sequence, &lexed_streamview); + + verilog::VerilogPreprocessData preprocessed_data = preprocessor.ScanStream(lexed_streamview); + auto& preprocessed_stream = preprocessed_data.preprocessed_token_stream; + for(auto u:preprocessed_stream) outs<<*u<<'\n'; // output the preprocessed tokens. + for(auto u:preprocessed_data.errors) outs< kCommands[] = { {"strip-comments", // {&StripComments, // @@ -82,6 +122,17 @@ static const std::pair kCommands[] = { Output: (stdout) Contents of original file with // and /**/ comments removed. )"}}, + + {"multiple-cu", // + {&MultipleCU, // + R"(multiple-cu file1 file2 ... + +Input: + 'file's are Verilog or SystemVerilog source files. + each one will be preprocessed in a separate compilation unit. +Output: (stdout) + Contents of original file with compiler directives interpreted. +)"}}, }; int main(int argc, char* argv[]) { From 07f2c7cb63c960d3b927adf3e293ac0a443c45b2 Mon Sep 17 00:00:00 2001 From: karimtera Date: Thu, 14 Jul 2022 01:14:13 +0200 Subject: [PATCH 02/17] constructing the control flow tree, to enable the preprocessor tool to generate all variants with the new mode generate-variants --- verilog/analysis/BUILD | 12 +++ verilog/analysis/flow_tree.cc | 86 +++++++++++++++++++ verilog/analysis/flow_tree.h | 45 ++++++++++ verilog/tools/preprocessor/BUILD | 2 + .../preprocessor/verilog_preprocessor.cc | 75 +++++++++++++++- 5 files changed, 217 insertions(+), 3 deletions(-) create mode 100644 verilog/analysis/flow_tree.cc create mode 100644 verilog/analysis/flow_tree.h diff --git a/verilog/analysis/BUILD b/verilog/analysis/BUILD index a95be9756..76ed6722b 100644 --- a/verilog/analysis/BUILD +++ b/verilog/analysis/BUILD @@ -74,6 +74,18 @@ cc_library( ], ) +cc_library( + name = "flow_tree", + srcs = ["flow_tree.cc"], + hdrs = ["flow_tree.h"], + deps = [ + "//common/lexer:token_stream_adapter", + "//verilog/parser:verilog_token_enum", + "@com_google_absl//absl/strings", + "@com_google_absl//absl/status", + ], +) + cc_library( name = "lint_rule_registry", srcs = ["lint_rule_registry.cc"], diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc new file mode 100644 index 000000000..de5773bb7 --- /dev/null +++ b/verilog/analysis/flow_tree.cc @@ -0,0 +1,86 @@ +// Copyright 2017-2020 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include +#include +#include +#include "absl/status/status.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" +#include "common/lexer/token_stream_adapter.h" +#include "verilog/parser/verilog_token_enum.h" +#include "verilog/analysis/flow_tree.h" + +namespace verilog { + +absl::Status FlowTree::GenerateControlFlowTree(){ + int idx=0; + int current_enum=0; + for(auto u:source_sequence_){ + current_enum=u.token_enum(); + + if(current_enum == PP_ifdef || current_enum == PP_ifndef){ + ifs_.push_back(idx); + elses_[ifs_.back()].push_back(idx); + + }else if(current_enum == PP_else || current_enum == PP_elsif || current_enum == PP_endif){ + elses_[ifs_.back()].push_back(idx); + if(current_enum == PP_endif){ + auto & myelses= elses_[ifs_.back()]; + for(int i=0;i0) edges_[idx-1].push_back(idx); + }else{ + if(idx>0) edges_[idx-1].push_back(edges_[idx].back()); + } + prv_enum = current_enum; + idx++; + } + + return absl::OkStatus(); +} + +absl::Status FlowTree::DepthFirstSearch(int index){ + const auto & curr=source_sequence_[index]; + if(curr.token_enum()!=PP_Identifier && curr.token_enum() != PP_ifndef && curr.token_enum()!=PP_ifdef + && curr.token_enum()!=PP_define && curr.token_enum()!=PP_define_body + && curr.token_enum()!=PP_elsif && curr.token_enum()!=PP_else && curr.token_enum()!=PP_endif) current_sequence_.push_back(curr); + for(auto u:edges_[index]){ + auto status = FlowTree::DepthFirstSearch(u); // handle errors + } + if(index==source_sequence_.size()-1){ + variants_.push_back(current_sequence_); + } + if(curr.token_enum()!=PP_Identifier && curr.token_enum() != PP_ifndef && curr.token_enum()!=PP_ifdef + && curr.token_enum()!=PP_define && curr.token_enum()!=PP_define_body + && curr.token_enum()!=PP_elsif && curr.token_enum()!=PP_else && curr.token_enum()!=PP_endif) current_sequence_.pop_back(); + return absl::OkStatus(); +} + +} // namespace verilog + diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h new file mode 100644 index 000000000..5502779c1 --- /dev/null +++ b/verilog/analysis/flow_tree.h @@ -0,0 +1,45 @@ +// Copyright 2017-2020 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef VERIBLE_VERILOG_FLOW_TREE_H_ +#define VERIBLE_VERILOG_FLOW_TREE_H_ + + +#include +#include +#include +#include "absl/status/status.h" +#include "common/lexer/token_stream_adapter.h" +#include "verilog/parser/verilog_token_enum.h" + +namespace verilog { + +class FlowTree { + public: + explicit FlowTree(verible::TokenSequence source_sequence): source_sequence_(std::move(source_sequence)){}; + + absl::Status GenerateControlFlowTree(); + absl::Status DepthFirstSearch(int index); + std::vector variants_; + private: + std::vector ifs_; + std::map> elses_; + std::map> edges_; + verible::TokenSequence source_sequence_; + verible::TokenSequence current_sequence_; +}; + +} // namespace verilog + +#endif // VERIBLE_VERILOG_FLOW_TREE_H_ diff --git a/verilog/tools/preprocessor/BUILD b/verilog/tools/preprocessor/BUILD index f41b865d7..3df875b7b 100644 --- a/verilog/tools/preprocessor/BUILD +++ b/verilog/tools/preprocessor/BUILD @@ -16,6 +16,8 @@ cc_binary( "//verilog/transform:strip_comments", "//verilog/preprocessor:verilog_preprocess", "//verilog/parser:verilog_lexer", + "//verilog/analysis:verilog_analyzer", + "//verilog/analysis:flow_tree", "@com_google_absl//absl/flags:usage", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 8aa0dd2e4..f975c917b 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -26,6 +26,10 @@ #include "verilog/transform/strip_comments.h" #include "verilog/parser/verilog_lexer.h" #include "verilog/preprocessor/verilog_preprocess.h" +#include "common/lexer/token_stream_adapter.h" +#include "verilog/analysis/verilog_analyzer.h" +#include "verilog/parser/verilog_token_enum.h" +#include "verilog/analysis/flow_tree.h" using verible::SubcommandArgsRange; using verible::SubcommandEntry; @@ -65,6 +69,7 @@ static absl::Status StripComments(const SubcommandArgsRange& args, return absl::OkStatus(); } + static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&, std::ostream& outs, std::ostream&) { @@ -81,6 +86,7 @@ static absl::Status MultipleCU(const SubcommandArgsRange& args, } verilog::VerilogPreprocess::Config config; config.filter_branches=1; + //config.expand_macros=1; verilog::VerilogPreprocess preprocessor(config); verilog::VerilogLexer lexer(source_contents); verible::TokenSequence lexed_sequence; @@ -88,18 +94,71 @@ static absl::Status MultipleCU(const SubcommandArgsRange& args, lexer.DoNextToken()) { // For now we will store the syntax tree tokens only, ignoring all the white-space characters. // however that should be stored to output the source code just like it was, but with conditionals filtered. - if(lexer.KeepSyntaxTreeTokens(lexer.GetLastToken())) lexed_sequence.push_back(lexer.GetLastToken()); + if(verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken())) lexed_sequence.push_back(lexer.GetLastToken()); } verible::TokenStreamView lexed_streamview; // Initializing the lexed token stream view. InitTokenStreamView(lexed_sequence, &lexed_streamview); - verilog::VerilogPreprocessData preprocessed_data = preprocessor.ScanStream(lexed_streamview); auto& preprocessed_stream = preprocessed_data.preprocessed_token_stream; for(auto u:preprocessed_stream) outs<<*u<<'\n'; // output the preprocessed tokens. - for(auto u:preprocessed_data.errors) outs<text()}; + std::string source_view{post_preproc}; + verilog::VerilogAnalyzer analyzer(source_view,"file1",config); + auto analyze_status = analyzer.Analyze(); + const auto& mydata = analyzer.Data().Contents(); + /* outs< Date: Thu, 14 Jul 2022 01:18:28 +0200 Subject: [PATCH 03/17] applying clang-format --- verilog/analysis/flow_tree.cc | 78 ++++++++-------- verilog/analysis/flow_tree.h | 16 ++-- .../preprocessor/verilog_preprocessor.cc | 91 ++++++++++--------- 3 files changed, 97 insertions(+), 88 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index de5773bb7..94920b23c 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -12,36 +12,39 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include +#include "verilog/analysis/flow_tree.h" + #include #include +#include + #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "common/lexer/token_stream_adapter.h" #include "verilog/parser/verilog_token_enum.h" -#include "verilog/analysis/flow_tree.h" namespace verilog { -absl::Status FlowTree::GenerateControlFlowTree(){ - int idx=0; - int current_enum=0; - for(auto u:source_sequence_){ - current_enum=u.token_enum(); +absl::Status FlowTree::GenerateControlFlowTree() { + int idx = 0; + int current_enum = 0; + for (auto u : source_sequence_) { + current_enum = u.token_enum(); - if(current_enum == PP_ifdef || current_enum == PP_ifndef){ + if (current_enum == PP_ifdef || current_enum == PP_ifndef) { ifs_.push_back(idx); elses_[ifs_.back()].push_back(idx); - }else if(current_enum == PP_else || current_enum == PP_elsif || current_enum == PP_endif){ + } else if (current_enum == PP_else || current_enum == PP_elsif || + current_enum == PP_endif) { elses_[ifs_.back()].push_back(idx); - if(current_enum == PP_endif){ - auto & myelses= elses_[ifs_.back()]; - for(int i=0;i0) edges_[idx-1].push_back(idx); - }else{ - if(idx>0) edges_[idx-1].push_back(edges_[idx].back()); + idx = 0; + int prv_enum = 0; + for (auto u : source_sequence_) { + current_enum = u.token_enum(); + if (current_enum != PP_else && current_enum != PP_elsif) { + if (idx > 0) edges_[idx - 1].push_back(idx); + } else { + if (idx > 0) edges_[idx - 1].push_back(edges_[idx].back()); } prv_enum = current_enum; idx++; @@ -65,22 +68,25 @@ absl::Status FlowTree::GenerateControlFlowTree(){ return absl::OkStatus(); } -absl::Status FlowTree::DepthFirstSearch(int index){ - const auto & curr=source_sequence_[index]; - if(curr.token_enum()!=PP_Identifier && curr.token_enum() != PP_ifndef && curr.token_enum()!=PP_ifdef - && curr.token_enum()!=PP_define && curr.token_enum()!=PP_define_body - && curr.token_enum()!=PP_elsif && curr.token_enum()!=PP_else && curr.token_enum()!=PP_endif) current_sequence_.push_back(curr); - for(auto u:edges_[index]){ - auto status = FlowTree::DepthFirstSearch(u); // handle errors +absl::Status FlowTree::DepthFirstSearch(int index) { + const auto& curr = source_sequence_[index]; + if (curr.token_enum() != PP_Identifier && curr.token_enum() != PP_ifndef && + curr.token_enum() != PP_ifdef && curr.token_enum() != PP_define && + curr.token_enum() != PP_define_body && curr.token_enum() != PP_elsif && + curr.token_enum() != PP_else && curr.token_enum() != PP_endif) + current_sequence_.push_back(curr); + for (auto u : edges_[index]) { + auto status = FlowTree::DepthFirstSearch(u); // handle errors } - if(index==source_sequence_.size()-1){ + if (index == source_sequence_.size() - 1) { variants_.push_back(current_sequence_); } - if(curr.token_enum()!=PP_Identifier && curr.token_enum() != PP_ifndef && curr.token_enum()!=PP_ifdef - && curr.token_enum()!=PP_define && curr.token_enum()!=PP_define_body - && curr.token_enum()!=PP_elsif && curr.token_enum()!=PP_else && curr.token_enum()!=PP_endif) current_sequence_.pop_back(); + if (curr.token_enum() != PP_Identifier && curr.token_enum() != PP_ifndef && + curr.token_enum() != PP_ifdef && curr.token_enum() != PP_define && + curr.token_enum() != PP_define_body && curr.token_enum() != PP_elsif && + curr.token_enum() != PP_else && curr.token_enum() != PP_endif) + current_sequence_.pop_back(); return absl::OkStatus(); } -} // namespace verilog - +} // namespace verilog diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index 5502779c1..03f6ffa38 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -15,10 +15,10 @@ #ifndef VERIBLE_VERILOG_FLOW_TREE_H_ #define VERIBLE_VERILOG_FLOW_TREE_H_ - -#include #include #include +#include + #include "absl/status/status.h" #include "common/lexer/token_stream_adapter.h" #include "verilog/parser/verilog_token_enum.h" @@ -27,19 +27,21 @@ namespace verilog { class FlowTree { public: - explicit FlowTree(verible::TokenSequence source_sequence): source_sequence_(std::move(source_sequence)){}; + explicit FlowTree(verible::TokenSequence source_sequence) + : source_sequence_(std::move(source_sequence)){}; absl::Status GenerateControlFlowTree(); absl::Status DepthFirstSearch(int index); std::vector variants_; + private: std::vector ifs_; - std::map> elses_; - std::map> edges_; + std::map> elses_; + std::map> edges_; verible::TokenSequence source_sequence_; verible::TokenSequence current_sequence_; }; -} // namespace verilog +} // namespace verilog -#endif // VERIBLE_VERILOG_FLOW_TREE_H_ +#endif // VERIBLE_VERILOG_FLOW_TREE_H_ diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index f975c917b..e02acb083 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -20,16 +20,16 @@ #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "common/lexer/token_stream_adapter.h" #include "common/util/file_util.h" #include "common/util/init_command_line.h" #include "common/util/subcommand.h" -#include "verilog/transform/strip_comments.h" -#include "verilog/parser/verilog_lexer.h" -#include "verilog/preprocessor/verilog_preprocess.h" -#include "common/lexer/token_stream_adapter.h" +#include "verilog/analysis/flow_tree.h" #include "verilog/analysis/verilog_analyzer.h" +#include "verilog/parser/verilog_lexer.h" #include "verilog/parser/verilog_token_enum.h" -#include "verilog/analysis/flow_tree.h" +#include "verilog/preprocessor/verilog_preprocess.h" +#include "verilog/transform/strip_comments.h" using verible::SubcommandArgsRange; using verible::SubcommandEntry; @@ -69,67 +69,68 @@ static absl::Status StripComments(const SubcommandArgsRange& args, return absl::OkStatus(); } - -static absl::Status MultipleCU(const SubcommandArgsRange& args, - std::istream&, std::ostream& outs, - std::ostream&) { +static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&, + std::ostream& outs, std::ostream&) { if (args.empty()) { - return absl::InvalidArgumentError( - "Missing file argument."); + return absl::InvalidArgumentError("Missing file argument."); } - for(auto source_file:args){ + for (auto source_file : args) { std::string source_contents; if (auto status = verible::file::GetContents(source_file, &source_contents); !status.ok()) { return status; } verilog::VerilogPreprocess::Config config; - config.filter_branches=1; - //config.expand_macros=1; + config.filter_branches = 1; + // config.expand_macros=1; verilog::VerilogPreprocess preprocessor(config); verilog::VerilogLexer lexer(source_contents); verible::TokenSequence lexed_sequence; - for (lexer.DoNextToken(); !lexer.GetLastToken().isEOF(); - lexer.DoNextToken()) { - // For now we will store the syntax tree tokens only, ignoring all the white-space characters. - // however that should be stored to output the source code just like it was, but with conditionals filtered. - if(verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken())) lexed_sequence.push_back(lexer.GetLastToken()); + for (lexer.DoNextToken(); !lexer.GetLastToken().isEOF(); + lexer.DoNextToken()) { + // For now we will store the syntax tree tokens only, ignoring all the + // white-space characters. however that should be stored to output the + // source code just like it was, but with conditionals filtered. + if (verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken())) + lexed_sequence.push_back(lexer.GetLastToken()); } verible::TokenStreamView lexed_streamview; // Initializing the lexed token stream view. InitTokenStreamView(lexed_sequence, &lexed_streamview); - verilog::VerilogPreprocessData preprocessed_data = preprocessor.ScanStream(lexed_streamview); + verilog::VerilogPreprocessData preprocessed_data = + preprocessor.ScanStream(lexed_streamview); auto& preprocessed_stream = preprocessed_data.preprocessed_token_stream; - for(auto u:preprocessed_stream) outs<<*u<<'\n'; // output the preprocessed tokens. - for(auto& u:preprocessed_data.errors) outs<text()}; + for (auto u : preprocessed_stream) post_preproc += std::string{u->text()}; std::string source_view{post_preproc}; - verilog::VerilogAnalyzer analyzer(source_view,"file1",config); + verilog::VerilogAnalyzer analyzer(source_view, "file1", config); auto analyze_status = analyzer.Analyze(); const auto& mydata = analyzer.Data().Contents(); /* outs< Date: Fri, 15 Jul 2022 18:18:39 +0200 Subject: [PATCH 04/17] fixing misc. errors in flow_tree.cc --- verilog/analysis/flow_tree.cc | 12 +++++------- verilog/tools/preprocessor/verilog_preprocessor.cc | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index 94920b23c..d9324306f 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -41,10 +41,10 @@ absl::Status FlowTree::GenerateControlFlowTree() { elses_[ifs_.back()].push_back(idx); if (current_enum == PP_endif) { auto& myelses = elses_[ifs_.back()]; - for (int i = 0; i < myelses.size(); i++) { - for (int j = i + 1; j < myelses.size(); j++) { - if (!i && j == myelses.size() - 1) continue; - edges_[myelses[i]].push_back(myelses[j] + 1); + for (auto it = myelses.begin(); it != myelses.end(); it++) { + for (auto it2 = it + 1; it2 != myelses.end(); it2++) { + if (it == myelses.begin() && it2 == myelses.end() - 1) continue; + edges_[*it].push_back(*it2 + (it2 != myelses.end() - 1)); } } ifs_.pop_back(); @@ -53,7 +53,6 @@ absl::Status FlowTree::GenerateControlFlowTree() { idx++; } idx = 0; - int prv_enum = 0; for (auto u : source_sequence_) { current_enum = u.token_enum(); if (current_enum != PP_else && current_enum != PP_elsif) { @@ -61,7 +60,6 @@ absl::Status FlowTree::GenerateControlFlowTree() { } else { if (idx > 0) edges_[idx - 1].push_back(edges_[idx].back()); } - prv_enum = current_enum; idx++; } @@ -78,7 +76,7 @@ absl::Status FlowTree::DepthFirstSearch(int index) { for (auto u : edges_[index]) { auto status = FlowTree::DepthFirstSearch(u); // handle errors } - if (index == source_sequence_.size() - 1) { + if (index == int(source_sequence_.size()) - 1) { variants_.push_back(current_sequence_); } if (curr.token_enum() != PP_Identifier && curr.token_enum() != PP_ifndef && diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index e02acb083..c5b3cf786 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -111,7 +111,7 @@ static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&, std::string source_view{post_preproc}; verilog::VerilogAnalyzer analyzer(source_view, "file1", config); auto analyze_status = analyzer.Analyze(); - const auto& mydata = analyzer.Data().Contents(); + /* const auto& mydata = analyzer.Data().Contents(); */ /* outs< Date: Fri, 22 Jul 2022 02:06:52 +0200 Subject: [PATCH 05/17] using ABSL flags in preprocessor tool, edited the tool test just to pass for the moment. --- .../preprocessor/verilog_preprocessor.cc | 108 ++++++++---------- .../preprocessor/verilog_preprocessor_test.sh | 67 +++++++++-- 2 files changed, 107 insertions(+), 68 deletions(-) diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index c5b3cf786..50eb825ea 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -16,6 +16,7 @@ #include #include +#include "absl/flags/flag.h" #include "absl/flags/usage.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" @@ -32,7 +33,6 @@ #include "verilog/transform/strip_comments.h" using verible::SubcommandArgsRange; -using verible::SubcommandEntry; static absl::Status StripComments(const SubcommandArgsRange& args, std::istream&, std::ostream& outs, @@ -69,71 +69,59 @@ static absl::Status StripComments(const SubcommandArgsRange& args, return absl::OkStatus(); } -static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&, +static absl::Status MultipleCU(const char* source_file, std::istream&, std::ostream& outs, std::ostream&) { - if (args.empty()) { - return absl::InvalidArgumentError("Missing file argument."); + std::string source_contents; + if (auto status = verible::file::GetContents(source_file, &source_contents); + !status.ok()) { + return status; } - - for (auto source_file : args) { - std::string source_contents; - if (auto status = verible::file::GetContents(source_file, &source_contents); - !status.ok()) { - return status; - } - verilog::VerilogPreprocess::Config config; - config.filter_branches = 1; - // config.expand_macros=1; - verilog::VerilogPreprocess preprocessor(config); - verilog::VerilogLexer lexer(source_contents); - verible::TokenSequence lexed_sequence; - for (lexer.DoNextToken(); !lexer.GetLastToken().isEOF(); - lexer.DoNextToken()) { - // For now we will store the syntax tree tokens only, ignoring all the - // white-space characters. however that should be stored to output the - // source code just like it was, but with conditionals filtered. - if (verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken())) - lexed_sequence.push_back(lexer.GetLastToken()); - } - verible::TokenStreamView lexed_streamview; - // Initializing the lexed token stream view. - InitTokenStreamView(lexed_sequence, &lexed_streamview); - verilog::VerilogPreprocessData preprocessed_data = - preprocessor.ScanStream(lexed_streamview); - auto& preprocessed_stream = preprocessed_data.preprocessed_token_stream; - for (auto u : preprocessed_stream) - outs << *u << '\n'; // output the preprocessed tokens. - for (auto& u : preprocessed_data.errors) - outs << u.error_message << '\n'; // for debugging. - // parsing just as a trial - std::string post_preproc; - for (auto u : preprocessed_stream) post_preproc += std::string{u->text()}; - std::string source_view{post_preproc}; - verilog::VerilogAnalyzer analyzer(source_view, "file1", config); - auto analyze_status = analyzer.Analyze(); - /* const auto& mydata = analyzer.Data().Contents(); */ - /* outs<text()}; + std::string source_view{post_preproc}; + verilog::VerilogAnalyzer analyzer(source_view, "file1", config); + auto analyze_status = analyzer.Analyze(); + /* const auto& mydata = analyzer.Data().Contents(); */ + /* outs< "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_OUTPUT_FILE" 2>&1 @@ -106,14 +132,12 @@ endmodule EOF cat > "$MY_EXPECT_FILE" < "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_OUTPUT_FILE" From b0f250e3a4db0b18f8f8f8d25d7149f55dd4e625 Mon Sep 17 00:00:00 2001 From: karimtera Date: Fri, 22 Jul 2022 16:31:14 +0200 Subject: [PATCH 06/17] adding usefull comments to flow_tree.cc --- verilog/analysis/flow_tree.cc | 151 ++++++++++++------ verilog/analysis/flow_tree.h | 25 ++- .../preprocessor/verilog_preprocessor.cc | 24 ++- 3 files changed, 138 insertions(+), 62 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index d9324306f..b9d0d4ca0 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -1,16 +1,17 @@ // Copyright 2017-2020 The Verible Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at +// you may not use this file except in compliance wedge_from_iteratorh the +// License. You may obtain a copy of the License at // // http://www.apache.org/licenses/LICENSE-2.0 // -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. +// Unless required by applicable law or agreed to in wredge_from_iteratoring, +// software distributed under the License is distributed on an "AS IS" BASIS, +// Wedge_from_iteratorHOUT WARRANTIES OR CONDedge_from_iteratorIONS OF ANY KIND, +// eedge_from_iteratorher express or implied. See the License for the specific +// language governing permissions and limedge_from_iteratorations under the +// License. #include "verilog/analysis/flow_tree.h" @@ -26,64 +27,120 @@ namespace verilog { +// Constructs the control flow tree, which determines the edge from each node +// (token index) to the next possible childs, And save edge_from_iterator in +// edges_. absl::Status FlowTree::GenerateControlFlowTree() { - int idx = 0; - int current_enum = 0; - for (auto u : source_sequence_) { - current_enum = u.token_enum(); + // Adding edges for if blocks. + int token_index = 0; + int current_token_enum = 0; + for (auto current_token : source_sequence_) { + current_token_enum = current_token.token_enum(); - if (current_enum == PP_ifdef || current_enum == PP_ifndef) { - ifs_.push_back(idx); - elses_[ifs_.back()].push_back(idx); + if (current_token_enum == PP_ifdef || current_token_enum == PP_ifndef) { + ifs_.push_back(token_index); // add index of `ifdef and `ifndef to ifs_. + elses_[ifs_.back()].push_back( + token_index); // also add edge_from_iterator to elses_ as if will + // help marking edges later on. - } else if (current_enum == PP_else || current_enum == PP_elsif || - current_enum == PP_endif) { - elses_[ifs_.back()].push_back(idx); - if (current_enum == PP_endif) { - auto& myelses = elses_[ifs_.back()]; - for (auto it = myelses.begin(); it != myelses.end(); it++) { - for (auto it2 = it + 1; it2 != myelses.end(); it2++) { - if (it == myelses.begin() && it2 == myelses.end() - 1) continue; - edges_[*it].push_back(*it2 + (it2 != myelses.end() - 1)); + } else if (current_token_enum == PP_else || + current_token_enum == PP_elsif || + current_token_enum == PP_endif) { + elses_[ifs_.back()].push_back( + token_index); // add index of `elsif, `else and `endif to else_ of + // the last recent if block. + if (current_token_enum == + PP_endif) { // if the current token is an `endif, then we are ready + // to create edges for this if block. + auto& current_if_block = + elses_[ifs_.back()]; // current_if_block contains all indexes of + // ifs and elses in the latest block. + + // Adding edges for each index in the if block using a nested loop. + for (auto edge_from_iterator = current_if_block.begin(); + edge_from_iterator != current_if_block.end(); + edge_from_iterator++) { + for (auto edge_to_iterator = edge_from_iterator + 1; + edge_to_iterator != current_if_block.end(); edge_to_iterator++) { + if (edge_from_iterator == current_if_block.begin() && + edge_to_iterator == current_if_block.end() - 1 && + current_if_block.size() > 2) + continue; // skip edges from `if to `endif if there is an else in + // this bloc wheneven there is an else in this block. + edges_[*edge_from_iterator].push_back( + *edge_to_iterator + + (edge_to_iterator != + current_if_block.end() - 1)); // add the possible edge. } } - ifs_.pop_back(); + ifs_.pop_back(); // the if block edges were added, ready to pop it. } } - idx++; + token_index++; // increment the token index. } - idx = 0; - for (auto u : source_sequence_) { - current_enum = u.token_enum(); - if (current_enum != PP_else && current_enum != PP_elsif) { - if (idx > 0) edges_[idx - 1].push_back(idx); + + // Adding edges for non-if blocks. + token_index = 0; + for (auto current_token : source_sequence_) { + current_token_enum = current_token.token_enum(); + if (current_token_enum != PP_else && current_token_enum != PP_elsif) { + if (token_index > 0) + edges_[token_index - 1].push_back( + token_index); // edges from a token to the one coming after it + // directly. } else { - if (idx > 0) edges_[idx - 1].push_back(edges_[idx].back()); + if (token_index > 0) + edges_[token_index - 1].push_back( + edges_[token_index] + .back()); // edges from the last token in `ifdef/`ifndef body + // to `endif from the same if block. } - idx++; + token_index++; // increment the token index. } return absl::OkStatus(); } -absl::Status FlowTree::DepthFirstSearch(int index) { - const auto& curr = source_sequence_[index]; - if (curr.token_enum() != PP_Identifier && curr.token_enum() != PP_ifndef && - curr.token_enum() != PP_ifdef && curr.token_enum() != PP_define && - curr.token_enum() != PP_define_body && curr.token_enum() != PP_elsif && - curr.token_enum() != PP_else && curr.token_enum() != PP_endif) - current_sequence_.push_back(curr); - for (auto u : edges_[index]) { - auto status = FlowTree::DepthFirstSearch(u); // handle errors +// Traveses the control flow tree in a depth first manner, appending the visited +// tokens to current_sequence_, then adding current_sequence_ to variants_ upon +// completing. +absl::Status FlowTree::DepthFirstSearch(int current_node_index) { + // skips preprocessor directives so that current_sequence_ doesn't contain + // any. + const auto& current_token = source_sequence_[current_node_index]; + if (current_token.token_enum() != PP_Identifier && + current_token.token_enum() != PP_ifndef && + current_token.token_enum() != PP_ifdef && + current_token.token_enum() != PP_define && + current_token.token_enum() != PP_define_body && + current_token.token_enum() != PP_elsif && + current_token.token_enum() != PP_else && + current_token.token_enum() != PP_endif) + current_sequence_.push_back(current_token); + + // do recursive search through every possible edge. + for (auto next_node : edges_[current_node_index]) { + if (auto status = FlowTree::DepthFirstSearch(next_node); !status.ok()) { + std::cerr << "ERROR: DepthFirstSearch fails\n"; + return status; + } } - if (index == int(source_sequence_.size()) - 1) { + if (current_node_index == + int(source_sequence_.size()) - + 1) { // if the current node is the last one, push the completed + // current_sequence_ to variants_. variants_.push_back(current_sequence_); } - if (curr.token_enum() != PP_Identifier && curr.token_enum() != PP_ifndef && - curr.token_enum() != PP_ifdef && curr.token_enum() != PP_define && - curr.token_enum() != PP_define_body && curr.token_enum() != PP_elsif && - curr.token_enum() != PP_else && curr.token_enum() != PP_endif) - current_sequence_.pop_back(); + if (current_token.token_enum() != PP_Identifier && + current_token.token_enum() != PP_ifndef && + current_token.token_enum() != PP_ifdef && + current_token.token_enum() != PP_define && + current_token.token_enum() != PP_define_body && + current_token.token_enum() != PP_elsif && + current_token.token_enum() != PP_else && + current_token.token_enum() != PP_endif) + current_sequence_ + .pop_back(); // remove tokens to back track into other variants. return absl::OkStatus(); } diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index 03f6ffa38..3de93c2bb 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -30,16 +30,25 @@ class FlowTree { explicit FlowTree(verible::TokenSequence source_sequence) : source_sequence_(std::move(source_sequence)){}; - absl::Status GenerateControlFlowTree(); - absl::Status DepthFirstSearch(int index); - std::vector variants_; + absl::Status GenerateControlFlowTree(); // constructs the control flow tree. + absl::Status DepthFirstSearch( + int index); // travese the tree in a depth first manner. + std::vector + variants_; // a memory for all variants generated. private: - std::vector ifs_; - std::map> elses_; - std::map> edges_; - verible::TokenSequence source_sequence_; - verible::TokenSequence current_sequence_; + std::vector + ifs_; // vector of all `ifdef/`ifndef indexes in source_sequence_. + std::map> elses_; // indexes of `elsif/`else indexes in + // source_sequence_ of each if block. + std::map> + edges_; // the tree edges which defines the possible next childs of each + // token in source_sequence_. + verible::TokenSequence + source_sequence_; // the original source code lexed token seqouence. + verible::TokenSequence + current_sequence_; // the variant's token sequence currrently being built + // by DepthFirstSearch. }; } // namespace verilog diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 50eb825ea..34d2993f2 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -74,6 +74,7 @@ static absl::Status MultipleCU(const char* source_file, std::istream&, std::string source_contents; if (auto status = verible::file::GetContents(source_file, &source_contents); !status.ok()) { + std::cerr << "ERROR: passed file can't be open\n."; return status; } verilog::VerilogPreprocess::Config config; @@ -125,8 +126,11 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, std::string source_contents; if (auto status = verible::file::GetContents(source_file, &source_contents); !status.ok()) { - return status; + std::cerr << "ERROR: passed file can't be open\n."; + return status; // check if the the file is not readable or doesn't exist. } + + // Lexing the input SV source code. verilog::VerilogLexer lexer(source_contents); verible::TokenSequence lexed_sequence; for (lexer.DoNextToken(); !lexer.GetLastToken().isEOF(); @@ -138,13 +142,19 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, lexed_sequence.push_back(lexer.GetLastToken()); } + // Control flow tree constructing. verilog::FlowTree control_flow_tree(lexed_sequence); - auto status = control_flow_tree.GenerateControlFlowTree(); - status = control_flow_tree.DepthFirstSearch(0); - int cnt = 1; - for (const auto& u : control_flow_tree.variants_) { - outs << "Variant number " << cnt++ << ":\n"; - for (auto k : u) outs << k << '\n'; + auto status = + control_flow_tree + .GenerateControlFlowTree(); // construct the control flow tree. + status = control_flow_tree.DepthFirstSearch( + 0); // traverse the tree by dfs from the root (node 0). + + // Printing the token streams of every possible variant. + int variants_counter = 1; + for (const auto& current_variant : control_flow_tree.variants_) { + outs << "Variant number " << variants_counter++ << ":\n"; + for (auto token : current_variant) outs << token << '\n'; puts(""); } From 0649948e2a4c305055bb457e0a5ffe5dd79b38f9 Mon Sep 17 00:00:00 2001 From: karimtera Date: Mon, 15 Aug 2022 02:34:20 +0200 Subject: [PATCH 07/17] Changes done: - Adding comments in a separate line to avoid wrapping. - Providing an API to the user of FlowTree class to generate variants. - Using const_iterators instead of index in FlowTree class. - Using a struct to represents the conditional block as a unit inside FlowTree. --- verilog/analysis/flow_tree.cc | 137 +++++++++--------- verilog/analysis/flow_tree.h | 53 ++++--- .../preprocessor/verilog_preprocessor.cc | 26 ++-- 3 files changed, 121 insertions(+), 95 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index b9d0d4ca0..0de87ac0d 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -1,4 +1,4 @@ -// Copyright 2017-2020 The Verible Authors. +// Copyright 2017-2022 The Verible Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance wedge_from_iteratorh the @@ -27,34 +27,40 @@ namespace verilog { +absl::Status FlowTree::GenerateVariants() { + return DepthFirstSearch(source_sequence_.begin()); +} + // Constructs the control flow tree, which determines the edge from each node // (token index) to the next possible childs, And save edge_from_iterator in // edges_. absl::Status FlowTree::GenerateControlFlowTree() { // Adding edges for if blocks. - int token_index = 0; int current_token_enum = 0; - for (auto current_token : source_sequence_) { - current_token_enum = current_token.token_enum(); + ConditionalBlock empty_conditional_block; + for (TokenSequenceConstIterator iter = source_sequence_.begin(); + iter != source_sequence_.end(); iter++) { + current_token_enum = iter->token_enum(); if (current_token_enum == PP_ifdef || current_token_enum == PP_ifndef) { - ifs_.push_back(token_index); // add index of `ifdef and `ifndef to ifs_. - elses_[ifs_.back()].push_back( - token_index); // also add edge_from_iterator to elses_ as if will - // help marking edges later on. + // Add iterator of `ifdef/`ifndef to if_blocks_. + if_blocks_.push_back(empty_conditional_block); + if_blocks_.back().if_block_begin = iter; + + // Also treat `ifdef/ifndef as an else, to help marking edges later on. + if_blocks_.back().else_block.push_back(iter); } else if (current_token_enum == PP_else || current_token_enum == PP_elsif || current_token_enum == PP_endif) { - elses_[ifs_.back()].push_back( - token_index); // add index of `elsif, `else and `endif to else_ of - // the last recent if block. - if (current_token_enum == - PP_endif) { // if the current token is an `endif, then we are ready - // to create edges for this if block. - auto& current_if_block = - elses_[ifs_.back()]; // current_if_block contains all indexes of - // ifs and elses in the latest block. + // Add iterator of `elsif/`else/`endif to else_block of the last recent if + // block. + if_blocks_.back().else_block.push_back(iter); + + // If the current token is an `endif, then we are ready to create edges + // for this if block. + if (current_token_enum == PP_endif) { + auto& current_if_block = if_blocks_.back().else_block; // Adding edges for each index in the if block using a nested loop. for (auto edge_from_iterator = current_if_block.begin(); @@ -62,40 +68,37 @@ absl::Status FlowTree::GenerateControlFlowTree() { edge_from_iterator++) { for (auto edge_to_iterator = edge_from_iterator + 1; edge_to_iterator != current_if_block.end(); edge_to_iterator++) { + // Skips edges from `if to `endif if there is `else in this block. if (edge_from_iterator == current_if_block.begin() && edge_to_iterator == current_if_block.end() - 1 && - current_if_block.size() > 2) - continue; // skip edges from `if to `endif if there is an else in - // this bloc wheneven there is an else in this block. + current_if_block.size() > 2) { + continue; + } + edges_[*edge_from_iterator].push_back( *edge_to_iterator + - (edge_to_iterator != - current_if_block.end() - 1)); // add the possible edge. + (edge_to_iterator != current_if_block.end() - 1)); } } - ifs_.pop_back(); // the if block edges were added, ready to pop it. + // The if block edges were added successfully, ready to pop it. + if_blocks_.pop_back(); } } - token_index++; // increment the token index. } // Adding edges for non-if blocks. - token_index = 0; - for (auto current_token : source_sequence_) { - current_token_enum = current_token.token_enum(); + for (TokenSequenceConstIterator iter = source_sequence_.begin(); + iter != source_sequence_.end(); iter++) { + current_token_enum = iter->token_enum(); if (current_token_enum != PP_else && current_token_enum != PP_elsif) { - if (token_index > 0) - edges_[token_index - 1].push_back( - token_index); // edges from a token to the one coming after it - // directly. + // Edges from a token to the one coming after it directly. + if (iter != source_sequence_.begin()) edges_[iter - 1].push_back(iter); + } else { - if (token_index > 0) - edges_[token_index - 1].push_back( - edges_[token_index] - .back()); // edges from the last token in `ifdef/`ifndef body - // to `endif from the same if block. + if (iter != source_sequence_.begin()) { + edges_[iter - 1].push_back(edges_[iter].back()); + } } - token_index++; // increment the token index. } return absl::OkStatus(); @@ -104,43 +107,43 @@ absl::Status FlowTree::GenerateControlFlowTree() { // Traveses the control flow tree in a depth first manner, appending the visited // tokens to current_sequence_, then adding current_sequence_ to variants_ upon // completing. -absl::Status FlowTree::DepthFirstSearch(int current_node_index) { - // skips preprocessor directives so that current_sequence_ doesn't contain - // any. - const auto& current_token = source_sequence_[current_node_index]; - if (current_token.token_enum() != PP_Identifier && - current_token.token_enum() != PP_ifndef && - current_token.token_enum() != PP_ifdef && - current_token.token_enum() != PP_define && - current_token.token_enum() != PP_define_body && - current_token.token_enum() != PP_elsif && - current_token.token_enum() != PP_else && - current_token.token_enum() != PP_endif) - current_sequence_.push_back(current_token); - - // do recursive search through every possible edge. - for (auto next_node : edges_[current_node_index]) { +absl::Status FlowTree::DepthFirstSearch( + TokenSequenceConstIterator current_node) { + // Skips directives so that current_sequence_ doesn't contain any. + if (current_node->token_enum() != PP_Identifier && + current_node->token_enum() != PP_ifndef && + current_node->token_enum() != PP_ifdef && + current_node->token_enum() != PP_define && + current_node->token_enum() != PP_define_body && + current_node->token_enum() != PP_elsif && + current_node->token_enum() != PP_else && + current_node->token_enum() != PP_endif) { + current_sequence_.push_back(*current_node); + } + + // Do recursive search through every possible edge. + for (auto next_node : edges_[current_node]) { if (auto status = FlowTree::DepthFirstSearch(next_node); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails\n"; return status; } } - if (current_node_index == - int(source_sequence_.size()) - - 1) { // if the current node is the last one, push the completed - // current_sequence_ to variants_. + // If the current node is the last one, push the completed current_sequence_ + // to variants_. + if (current_node == source_sequence_.end() - 1) { variants_.push_back(current_sequence_); } - if (current_token.token_enum() != PP_Identifier && - current_token.token_enum() != PP_ifndef && - current_token.token_enum() != PP_ifdef && - current_token.token_enum() != PP_define && - current_token.token_enum() != PP_define_body && - current_token.token_enum() != PP_elsif && - current_token.token_enum() != PP_else && - current_token.token_enum() != PP_endif) - current_sequence_ - .pop_back(); // remove tokens to back track into other variants. + if (current_node->token_enum() != PP_Identifier && + current_node->token_enum() != PP_ifndef && + current_node->token_enum() != PP_ifdef && + current_node->token_enum() != PP_define && + current_node->token_enum() != PP_define_body && + current_node->token_enum() != PP_elsif && + current_node->token_enum() != PP_else && + current_node->token_enum() != PP_endif) { + // Remove tokens to back track into other variants. + current_sequence_.pop_back(); + } return absl::OkStatus(); } diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index 3de93c2bb..b54ef0abf 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -1,4 +1,4 @@ -// Copyright 2017-2020 The Verible Authors. +// Copyright 2017-2022 The Verible Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ #define VERIBLE_VERILOG_FLOW_TREE_H_ #include +#include #include #include @@ -24,31 +25,47 @@ #include "verilog/parser/verilog_token_enum.h" namespace verilog { +using TokenSequenceConstIterator = verible::TokenSequence::const_iterator; + +struct ConditionalBlock { + // Start of a ifdef/ifndef block + TokenSequenceConstIterator if_block_begin; + + // Subsequent else/elsif blocks + std::vector else_block; +}; class FlowTree { public: explicit FlowTree(verible::TokenSequence source_sequence) : source_sequence_(std::move(source_sequence)){}; - absl::Status GenerateControlFlowTree(); // constructs the control flow tree. - absl::Status DepthFirstSearch( - int index); // travese the tree in a depth first manner. - std::vector - variants_; // a memory for all variants generated. + // Constructs the control flow tree by adding the tree edges in edges_. + absl::Status GenerateControlFlowTree(); + + // Generates all possible variants. + absl::Status GenerateVariants(); + + // A memory for all variants generated. + std::vector variants_; private: - std::vector - ifs_; // vector of all `ifdef/`ifndef indexes in source_sequence_. - std::map> elses_; // indexes of `elsif/`else indexes in - // source_sequence_ of each if block. - std::map> - edges_; // the tree edges which defines the possible next childs of each - // token in source_sequence_. - verible::TokenSequence - source_sequence_; // the original source code lexed token seqouence. - verible::TokenSequence - current_sequence_; // the variant's token sequence currrently being built - // by DepthFirstSearch. + // Traveses the tree in a depth first manner. + absl::Status DepthFirstSearch(TokenSequenceConstIterator current_node); + + // The tree edges which defines the possible next childs of each token in + // source_sequence_. + std::map> + edges_; + + // Holds all of the conditional blocks. + std::vector if_blocks_; + + // The original source code lexed token seqouence. + const verible::TokenSequence source_sequence_; + + // The variant's token sequence currrently being built by DepthFirstSearch. + verible::TokenSequence current_sequence_; }; } // namespace verilog diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 34d2993f2..71eefaafc 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -1,4 +1,4 @@ -// Copyright 2017-2020 The Verible Authors. +// Copyright 2017-2022 The Verible Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -97,10 +97,10 @@ static absl::Status MultipleCU(const char* source_file, std::istream&, verilog::VerilogPreprocessData preprocessed_data = preprocessor.ScanStream(lexed_streamview); auto& preprocessed_stream = preprocessed_data.preprocessed_token_stream; - for (auto u : preprocessed_stream) - outs << *u << '\n'; // output the preprocessed tokens. - for (auto& u : preprocessed_data.errors) - outs << u.error_message << '\n'; // for debugging. + for (auto u : preprocessed_stream) outs << *u << '\n'; + // for debugging. + for (auto& u : preprocessed_data.errors) outs << u.error_message << '\n'; + // parsing just as a trial std::string post_preproc; for (auto u : preprocessed_stream) post_preproc += std::string{u->text()}; @@ -144,11 +144,17 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, // Control flow tree constructing. verilog::FlowTree control_flow_tree(lexed_sequence); - auto status = - control_flow_tree - .GenerateControlFlowTree(); // construct the control flow tree. - status = control_flow_tree.DepthFirstSearch( - 0); // traverse the tree by dfs from the root (node 0). + + auto status = control_flow_tree.GenerateControlFlowTree(); + if (!status.ok()) { + std::cerr << "ERROR:: couldn't generate the control flow tree.\n"; + return status; + } + status = control_flow_tree.GenerateVariants(); + if (!status.ok()) { + std::cerr << "ERROR:: couldn't generate variants.\n"; + return status; + } // Printing the token streams of every possible variant. int variants_counter = 1; From 357d7292f73b3c38008fe2ce8f554ccd046cdd7b Mon Sep 17 00:00:00 2001 From: karimtera Date: Tue, 16 Aug 2022 22:36:48 +0200 Subject: [PATCH 08/17] FlowTree class: Provided a callback function to the user, and removed variants_. --- verilog/analysis/flow_tree.cc | 21 +++++++++------- verilog/analysis/flow_tree.h | 22 +++++++++++++---- .../preprocessor/verilog_preprocessor.cc | 24 ++++++++++++------- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index 0de87ac0d..ae8fef3d1 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -27,8 +27,8 @@ namespace verilog { -absl::Status FlowTree::GenerateVariants() { - return DepthFirstSearch(source_sequence_.begin()); +absl::Status FlowTree::GenerateVariants(const VariantReceiver &receiver) { + return DepthFirstSearch(receiver, source_sequence_.begin()); } // Constructs the control flow tree, which determines the edge from each node @@ -60,7 +60,7 @@ absl::Status FlowTree::GenerateControlFlowTree() { // If the current token is an `endif, then we are ready to create edges // for this if block. if (current_token_enum == PP_endif) { - auto& current_if_block = if_blocks_.back().else_block; + const auto ¤t_if_block = if_blocks_.back().else_block; // Adding edges for each index in the if block using a nested loop. for (auto edge_from_iterator = current_if_block.begin(); @@ -105,10 +105,13 @@ absl::Status FlowTree::GenerateControlFlowTree() { } // Traveses the control flow tree in a depth first manner, appending the visited -// tokens to current_sequence_, then adding current_sequence_ to variants_ upon -// completing. +// tokens to current_sequence_, then provide the completed variant to the user +// using a callback function (VariantReceiver). absl::Status FlowTree::DepthFirstSearch( - TokenSequenceConstIterator current_node) { + const VariantReceiver &receiver, TokenSequenceConstIterator current_node) { + if (!receiver(current_sequence_, variants_counter_, false)) { + return absl::OkStatus(); + } // Skips directives so that current_sequence_ doesn't contain any. if (current_node->token_enum() != PP_Identifier && current_node->token_enum() != PP_ifndef && @@ -123,7 +126,8 @@ absl::Status FlowTree::DepthFirstSearch( // Do recursive search through every possible edge. for (auto next_node : edges_[current_node]) { - if (auto status = FlowTree::DepthFirstSearch(next_node); !status.ok()) { + if (auto status = FlowTree::DepthFirstSearch(receiver, next_node); + !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails\n"; return status; } @@ -131,7 +135,8 @@ absl::Status FlowTree::DepthFirstSearch( // If the current node is the last one, push the completed current_sequence_ // to variants_. if (current_node == source_sequence_.end() - 1) { - variants_.push_back(current_sequence_); + receiver(current_sequence_, variants_counter_, true); + variants_counter_++; } if (current_node->token_enum() != PP_Identifier && current_node->token_enum() != PP_ifndef && diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index b54ef0abf..6accf09d6 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -27,6 +27,13 @@ namespace verilog { using TokenSequenceConstIterator = verible::TokenSequence::const_iterator; +// Receive a complete token sequence of one variant. +// 1st argument: generated variant. +// 2nd argument: number of generated variants (to decide if more is needed). +// 3rd argument: checks if the variant is ready to be sent. +using VariantReceiver = + std::function; + struct ConditionalBlock { // Start of a ifdef/ifndef block TokenSequenceConstIterator if_block_begin; @@ -35,6 +42,10 @@ struct ConditionalBlock { std::vector else_block; }; +// FlowTree class builds the control flow graph of a tokenized System-Verilog +// source code. Furthermore, enabling doing the following queries on the graph: +// 1) Generating all the possible variants (provided via a callback function). +// 2) TODO(karimtera): uniquely identify a variant with a bitset. class FlowTree { public: explicit FlowTree(verible::TokenSequence source_sequence) @@ -44,14 +55,12 @@ class FlowTree { absl::Status GenerateControlFlowTree(); // Generates all possible variants. - absl::Status GenerateVariants(); - - // A memory for all variants generated. - std::vector variants_; + absl::Status GenerateVariants(const VariantReceiver &receiver); private: // Traveses the tree in a depth first manner. - absl::Status DepthFirstSearch(TokenSequenceConstIterator current_node); + absl::Status DepthFirstSearch(const VariantReceiver &receiver, + TokenSequenceConstIterator current_node); // The tree edges which defines the possible next childs of each token in // source_sequence_. @@ -66,6 +75,9 @@ class FlowTree { // The variant's token sequence currrently being built by DepthFirstSearch. verible::TokenSequence current_sequence_; + + // Number of vairants generated so far. + int variants_counter_ = 0; }; } // namespace verilog diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 71eefaafc..c70636883 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -34,6 +34,20 @@ using verible::SubcommandArgsRange; +bool PrintGeneratedVariant(const verible::TokenSequence& recieved_variant, + const int variants_counter, + const bool is_variant_complete) { + if (!is_variant_complete) { + // Only accepts 2 variants at most. (to test the functionality). + // TODO(karimtera): How many variants are practical? + return variants_counter < 2; + } + std::cout << "Variant number " << variants_counter + 1 << ":\n"; + for (auto token : recieved_variant) std::cout << token << '\n'; + puts(""); + return true; +} + static absl::Status StripComments(const SubcommandArgsRange& args, std::istream&, std::ostream& outs, std::ostream&) { @@ -150,20 +164,12 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, std::cerr << "ERROR:: couldn't generate the control flow tree.\n"; return status; } - status = control_flow_tree.GenerateVariants(); + status = control_flow_tree.GenerateVariants(PrintGeneratedVariant); if (!status.ok()) { std::cerr << "ERROR:: couldn't generate variants.\n"; return status; } - // Printing the token streams of every possible variant. - int variants_counter = 1; - for (const auto& current_variant : control_flow_tree.variants_) { - outs << "Variant number " << variants_counter++ << ":\n"; - for (auto token : current_variant) outs << token << '\n'; - puts(""); - } - return absl::OkStatus(); } From 7e5419ed3cc1965177bd15e0af89126ebd1dee23 Mon Sep 17 00:00:00 2001 From: karimtera Date: Wed, 17 Aug 2022 17:08:59 +0200 Subject: [PATCH 09/17] FlowTree class: - Listed the macros as they appear in conditionals. - Gave each of them a unique ID and stored them in a map. - Used a more detailed if_blocks struct to help tracking edges. - Used bitset in DFS to know for each variant which macros are defined. --- verilog/analysis/flow_tree.cc | 315 ++++++++++++++---- verilog/analysis/flow_tree.h | 41 ++- .../preprocessor/verilog_preprocessor.cc | 2 +- 3 files changed, 290 insertions(+), 68 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index ae8fef3d1..2bb8b9f0d 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -27,8 +27,149 @@ namespace verilog { +// Adds edges within a conditonal block. +// Such that the first edge represents the condition being true, +// and the second edge represents the condition being false. +absl::Status FlowTree::AddBlockEdges(const ConditionalBlock &block) { + bool contains_elsif = !block.elsif_iterators.empty(); + bool contains_else = block.else_iterator != source_sequence_.end(); + + // Handling `ifdef/ifndef. + auto ifdef_or_ifndef = + (block.ifdef_iterator == source_sequence_.end() ? block.ifndef_iterator + : block.ifdef_iterator); + + // Assuming the condition is true. + edges_[ifdef_or_ifndef].push_back(ifdef_or_ifndef + 1); + + // Assuming the condition is false. + // Checking if there is an `elsif. + if (contains_elsif) { + // Add edge to the first `elsif in the block. + edges_[ifdef_or_ifndef].push_back(block.elsif_iterators[0]); + } else if (contains_else) { + // Checking if there is an `else. + edges_[ifdef_or_ifndef].push_back(block.else_iterator); + } else { + // `endif exists. + edges_[ifdef_or_ifndef].push_back(block.endif_iterator); + } + + // Handling `elsif. + if (contains_elsif) { + for (auto iter = block.elsif_iterators.begin(); + iter != block.elsif_iterators.end(); iter++) { + // Assuming the condition is true. + edges_[*iter].push_back((*iter) + 1); + + // Assuming the condition is false. + if (iter + 1 != block.elsif_iterators.end()) + edges_[*iter].push_back(*(iter + 1)); + else if (contains_else) + edges_[*iter].push_back(block.else_iterator); + else + edges_[*iter].push_back(block.endif_iterator); + } + } + + // Handling `else. + if (contains_else) { + edges_[block.else_iterator].push_back(block.else_iterator + 1); + } + + // For edges that are generated assuming the conditons are true, + // We need to add an edge from the end of the condition group of lines to + // `endif, e.g. `ifdef + // + // + // ... + // + // `else + // + // `endif + // Edge to be added: from to `endif. + edges_[block.endif_iterator - 1].push_back(block.endif_iterator); + if (contains_elsif) { + for (auto iter : block.elsif_iterators) + edges_[iter - 1].push_back(block.endif_iterator); + } else if (contains_else) { + edges_[block.else_iterator - 1].push_back(block.endif_iterator); + } + + // Connecting `endif to the next token directly (if not EOF). + auto next_iter = block.endif_iterator + 1; + if (next_iter != source_sequence_.end() && + next_iter->token_enum() != PP_else && + next_iter->token_enum() != PP_elsif && + next_iter->token_enum() != PP_endif) { + edges_[block.endif_iterator].push_back(next_iter); + } + + return absl::OkStatus(); +} + +// Checks if the iterator is pointing to a conditional directive. +bool FlowTree::IsConditional(TokenSequenceConstIterator iterator) { + auto current_node = iterator->token_enum(); + return current_node == PP_ifndef || current_node == PP_ifdef || + current_node == PP_elsif || current_node == PP_else || + current_node == PP_endif; +} + +// Checks if after the conditional_iterator (`ifdef/`ifndef... ) there exists +// a macro identifier. +absl::Status FlowTree::MacroFollows( + TokenSequenceConstIterator conditional_iterator) { + if (conditional_iterator->token_enum() != PP_ifdef && + conditional_iterator->token_enum() != PP_ifndef && + conditional_iterator->token_enum() != PP_elsif) { + return absl::InvalidArgumentError("Error macro name can't be extracted."); + } + auto macro_iterator = conditional_iterator + 1; + if (macro_iterator->token_enum() != PP_Identifier) + return absl::InvalidArgumentError("Expected identifier for macro name."); + else + return absl::OkStatus(); +} + +// Adds a conditional macro to conditional_macro_id_ if not added before, +// And gives it a new ID. +absl::Status FlowTree::AddMacroOfConditionalToMap( + TokenSequenceConstIterator conditional_iterator) { + auto status = MacroFollows(conditional_iterator); + if (!status.ok()) { + return absl::InvalidArgumentError( + "Error no macro follows the conditional directive."); + } + auto macro_iterator = conditional_iterator + 1; + auto macro_identifier = macro_iterator->text(); + if (conditional_macro_id_.find(macro_identifier) == + conditional_macro_id_.end()) { + conditional_macros_counter++; + conditional_macro_id_[macro_identifier] = conditional_macros_counter; + } + return absl::OkStatus(); +} + +// Gets the conditonal macro ID from the conditional_macro_id_. +// Note: conditional_iterator is pointing to the conditional. +int FlowTree::GetMacroIDOfConditional( + TokenSequenceConstIterator conditional_iterator) { + auto status = MacroFollows(conditional_iterator); + if (!status.ok()) { + // TODO(karimtera): add a better error handling. + return -1; + } + auto macro_iterator = conditional_iterator + 1; + auto macro_identifier = macro_iterator->text(); + // It is always assumed that the macro already exists in the map. + return conditional_macro_id_[macro_identifier]; +} + +// An API that provides a callback function to receive variants. absl::Status FlowTree::GenerateVariants(const VariantReceiver &receiver) { - return DepthFirstSearch(receiver, source_sequence_.begin()); + std::bitset<128> assumed; + return DepthFirstSearch(receiver, source_sequence_.begin(), assumed); } // Constructs the control flow tree, which determines the edge from each node @@ -37,66 +178,69 @@ absl::Status FlowTree::GenerateVariants(const VariantReceiver &receiver) { absl::Status FlowTree::GenerateControlFlowTree() { // Adding edges for if blocks. int current_token_enum = 0; - ConditionalBlock empty_conditional_block; + ConditionalBlock empty_block; + empty_block.ifdef_iterator = source_sequence_.end(); + empty_block.ifndef_iterator = source_sequence_.end(); + empty_block.else_iterator = source_sequence_.end(); + empty_block.endif_iterator = source_sequence_.end(); for (TokenSequenceConstIterator iter = source_sequence_.begin(); iter != source_sequence_.end(); iter++) { current_token_enum = iter->token_enum(); - if (current_token_enum == PP_ifdef || current_token_enum == PP_ifndef) { - // Add iterator of `ifdef/`ifndef to if_blocks_. - if_blocks_.push_back(empty_conditional_block); - if_blocks_.back().if_block_begin = iter; - - // Also treat `ifdef/ifndef as an else, to help marking edges later on. - if_blocks_.back().else_block.push_back(iter); - - } else if (current_token_enum == PP_else || - current_token_enum == PP_elsif || - current_token_enum == PP_endif) { - // Add iterator of `elsif/`else/`endif to else_block of the last recent if - // block. - if_blocks_.back().else_block.push_back(iter); - - // If the current token is an `endif, then we are ready to create edges - // for this if block. - if (current_token_enum == PP_endif) { - const auto ¤t_if_block = if_blocks_.back().else_block; - - // Adding edges for each index in the if block using a nested loop. - for (auto edge_from_iterator = current_if_block.begin(); - edge_from_iterator != current_if_block.end(); - edge_from_iterator++) { - for (auto edge_to_iterator = edge_from_iterator + 1; - edge_to_iterator != current_if_block.end(); edge_to_iterator++) { - // Skips edges from `if to `endif if there is `else in this block. - if (edge_from_iterator == current_if_block.begin() && - edge_to_iterator == current_if_block.end() - 1 && - current_if_block.size() > 2) { - continue; - } - - edges_[*edge_from_iterator].push_back( - *edge_to_iterator + - (edge_to_iterator != current_if_block.end() - 1)); + + if (IsConditional(iter)) { + switch (current_token_enum) { + case PP_ifdef: { + if_blocks_.push_back(empty_block); + if_blocks_.back().ifdef_iterator = iter; + auto status = AddMacroOfConditionalToMap(iter); + if (!status.ok()) { + return absl::InvalidArgumentError( + "Error couldn't give a macro an ID."); } + break; + } + case PP_ifndef: { + if_blocks_.push_back(empty_block); + if_blocks_.back().ifndef_iterator = iter; + auto status = AddMacroOfConditionalToMap(iter); + if (!status.ok()) { + return absl::InvalidArgumentError( + "Error couldn't give a macro an ID."); + } + break; + } + case PP_elsif: { + if_blocks_.back().elsif_iterators.push_back(iter); + auto status = AddMacroOfConditionalToMap(iter); + if (!status.ok()) { + return absl::InvalidArgumentError( + "Error couldn't give a macro an ID."); + } + break; + } + case PP_else: { + if_blocks_.back().else_iterator = iter; + break; + } + case PP_endif: { + if_blocks_.back().endif_iterator = iter; + auto status = AddBlockEdges(if_blocks_.back()); + if (!status.ok()) return status; + // TODO(karimtera): add an error message. + if_blocks_.pop_back(); + break; } - // The if block edges were added successfully, ready to pop it. - if_blocks_.pop_back(); } - } - } - - // Adding edges for non-if blocks. - for (TokenSequenceConstIterator iter = source_sequence_.begin(); - iter != source_sequence_.end(); iter++) { - current_token_enum = iter->token_enum(); - if (current_token_enum != PP_else && current_token_enum != PP_elsif) { - // Edges from a token to the one coming after it directly. - if (iter != source_sequence_.begin()) edges_[iter - 1].push_back(iter); } else { - if (iter != source_sequence_.begin()) { - edges_[iter - 1].push_back(edges_[iter].back()); + // Only add normal edges if the next token is not `else/`elsif/`endif. + auto next_iter = iter + 1; + if (next_iter != source_sequence_.end() && + next_iter->token_enum() != PP_else && + next_iter->token_enum() != PP_elsif && + next_iter->token_enum() != PP_endif) { + edges_[iter].push_back(next_iter); } } } @@ -107,8 +251,9 @@ absl::Status FlowTree::GenerateControlFlowTree() { // Traveses the control flow tree in a depth first manner, appending the visited // tokens to current_sequence_, then provide the completed variant to the user // using a callback function (VariantReceiver). -absl::Status FlowTree::DepthFirstSearch( - const VariantReceiver &receiver, TokenSequenceConstIterator current_node) { +absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, + TokenSequenceConstIterator current_node, + std::bitset<128> assumed) { if (!receiver(current_sequence_, variants_counter_, false)) { return absl::OkStatus(); } @@ -124,16 +269,64 @@ absl::Status FlowTree::DepthFirstSearch( current_sequence_.push_back(*current_node); } - // Do recursive search through every possible edge. - for (auto next_node : edges_[current_node]) { - if (auto status = FlowTree::DepthFirstSearch(receiver, next_node); - !status.ok()) { - std::cerr << "ERROR: DepthFirstSearch fails\n"; - return status; + // Checks if the current token is a `ifdef/`ifndef/`elsif. + if (current_node->token_enum() == PP_ifdef || + current_node->token_enum() == PP_ifndef || + current_node->token_enum() == PP_elsif) { + int macro_id = GetMacroIDOfConditional(current_node); + bool negated = (current_node->token_enum() == PP_ifndef); + // Checks if this macro is already assumed to be defined/undefined. + if (assumed.test(macro_id)) { + bool assume_condition_is_true = + (negated ^ current_macros_.test(macro_id)); + if (auto status = DepthFirstSearch( + receiver, edges_[current_node][!assume_condition_is_true], + assumed); + !status.ok()) { + std::cerr << "ERROR: DepthFirstSearch fails."; + return status; + } + } else { + assumed.flip(macro_id); + // This macro wans't assumed before, then we can check both edges. + // Assume the condition is true. + if (negated) + current_macros_.reset(macro_id); + else + current_macros_.set(macro_id); + if (auto status = + DepthFirstSearch(receiver, edges_[current_node][0], assumed); + !status.ok()) { + std::cerr << "ERROR: DepthFirstSearch fails."; + return status; + } + + // Assume the condition is false. + if (!negated) + current_macros_.reset(macro_id); + else + current_macros_.set(macro_id); + if (auto status = + DepthFirstSearch(receiver, edges_[current_node][1], assumed); + !status.ok()) { + std::cerr << "ERROR: DepthFirstSearch fails."; + return status; + } + } + } else { + // Do recursive search through every possible edge. + // Expected to be only one edge in this case. + for (auto next_node : edges_[current_node]) { + if (auto status = + FlowTree::DepthFirstSearch(receiver, next_node, assumed); + !status.ok()) { + std::cerr << "ERROR: DepthFirstSearch fails\n"; + return status; + } } } // If the current node is the last one, push the completed current_sequence_ - // to variants_. + // then it is ready to be sent. if (current_node == source_sequence_.end() - 1) { receiver(current_sequence_, variants_counter_, true); variants_counter_++; diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index 6accf09d6..186698d4a 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -15,6 +15,7 @@ #ifndef VERIBLE_VERILOG_FLOW_TREE_H_ #define VERIBLE_VERILOG_FLOW_TREE_H_ +#include #include #include #include @@ -35,11 +36,11 @@ using VariantReceiver = std::function; struct ConditionalBlock { - // Start of a ifdef/ifndef block - TokenSequenceConstIterator if_block_begin; - - // Subsequent else/elsif blocks - std::vector else_block; + TokenSequenceConstIterator ifdef_iterator; + TokenSequenceConstIterator ifndef_iterator; + std::vector elsif_iterators; + TokenSequenceConstIterator else_iterator; + TokenSequenceConstIterator endif_iterator; }; // FlowTree class builds the control flow graph of a tokenized System-Verilog @@ -60,13 +61,30 @@ class FlowTree { private: // Traveses the tree in a depth first manner. absl::Status DepthFirstSearch(const VariantReceiver &receiver, - TokenSequenceConstIterator current_node); + TokenSequenceConstIterator current_node, + std::bitset<128> assumed); + + // Checks if the iterator points to a conditonal directive (`ifdef/ifndef...). + static bool IsConditional(TokenSequenceConstIterator iterator); + + // + absl::Status AddBlockEdges(const ConditionalBlock &block); // The tree edges which defines the possible next childs of each token in // source_sequence_. std::map> edges_; + // Extracts the conditional macro checked. + static absl::Status MacroFollows( + TokenSequenceConstIterator conditional_iterator); + + // Adds macro to conditional_macro_id_ map. + absl::Status AddMacroOfConditionalToMap( + TokenSequenceConstIterator conditional_iterator); + + int GetMacroIDOfConditional(TokenSequenceConstIterator conditional_iterator); + // Holds all of the conditional blocks. std::vector if_blocks_; @@ -76,6 +94,17 @@ class FlowTree { // The variant's token sequence currrently being built by DepthFirstSearch. verible::TokenSequence current_sequence_; + // Macros assumed in the variant that is currently begin built. + // TODO(karimtera): Is it better to use dymanic_bitset? + std::bitset<128> current_macros_; + + // Mapping each conditional macro to an integer ID, + // to use it later as a bit offset. + std::map conditional_macro_id_; + + // Number of macros appeared in `ifdef/`ifndef/`elsif. + int conditional_macros_counter = 0; + // Number of vairants generated so far. int variants_counter_ = 0; }; diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index c70636883..43f27d025 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -40,7 +40,7 @@ bool PrintGeneratedVariant(const verible::TokenSequence& recieved_variant, if (!is_variant_complete) { // Only accepts 2 variants at most. (to test the functionality). // TODO(karimtera): How many variants are practical? - return variants_counter < 2; + return variants_counter < 20; } std::cout << "Variant number " << variants_counter + 1 << ":\n"; for (auto token : recieved_variant) std::cout << token << '\n'; From c0ed68e469927754f479e8ffaaf807a2be03b808 Mon Sep 17 00:00:00 2001 From: karimtera Date: Thu, 18 Aug 2022 02:29:59 +0200 Subject: [PATCH 10/17] FlowTree class: - Removed variants counter from the class members. - Added a new struct called Variant that contains all the vairant's data: - Its TokenSequence. - bitset that shows if the i-th macro is visited/assumed or not. - bitset that shows if the i-th macro is defined or not. - Modified the VariantReceiver to accepts the new struct Variant, instead of just TokenSequence. - GenerateControlFlowTree() is now a private function, such that GenerateVariants() is the only API the user needs. - Added a using declaration for std::bitset<128>. --- verilog/analysis/flow_tree.cc | 50 ++++++++++--------- verilog/analysis/flow_tree.h | 50 +++++++++++-------- .../preprocessor/verilog_preprocessor.cc | 26 ++++------ 3 files changed, 64 insertions(+), 62 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index 2bb8b9f0d..f2be1dc30 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -145,8 +145,8 @@ absl::Status FlowTree::AddMacroOfConditionalToMap( auto macro_identifier = macro_iterator->text(); if (conditional_macro_id_.find(macro_identifier) == conditional_macro_id_.end()) { - conditional_macros_counter++; - conditional_macro_id_[macro_identifier] = conditional_macros_counter; + conditional_macro_id_[macro_identifier] = conditional_macros_counter_; + conditional_macros_counter_++; } return absl::OkStatus(); } @@ -168,8 +168,12 @@ int FlowTree::GetMacroIDOfConditional( // An API that provides a callback function to receive variants. absl::Status FlowTree::GenerateVariants(const VariantReceiver &receiver) { - std::bitset<128> assumed; - return DepthFirstSearch(receiver, source_sequence_.begin(), assumed); + auto status = GenerateControlFlowTree(); + if (!status.ok()) { + return absl::InvalidArgumentError( + "Error: creating the control flow graph has failed."); + } + return DepthFirstSearch(receiver, source_sequence_.begin(), 1); } // Constructs the control flow tree, which determines the edge from each node @@ -253,10 +257,9 @@ absl::Status FlowTree::GenerateControlFlowTree() { // using a callback function (VariantReceiver). absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, TokenSequenceConstIterator current_node, - std::bitset<128> assumed) { - if (!receiver(current_sequence_, variants_counter_, false)) { - return absl::OkStatus(); - } + bool wants_more) { + if (!wants_more) return absl::OkStatus(); + // Skips directives so that current_sequence_ doesn't contain any. if (current_node->token_enum() != PP_Identifier && current_node->token_enum() != PP_ifndef && @@ -266,7 +269,7 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, current_node->token_enum() != PP_elsif && current_node->token_enum() != PP_else && current_node->token_enum() != PP_endif) { - current_sequence_.push_back(*current_node); + current_variant_.sequence.push_back(*current_node); } // Checks if the current token is a `ifdef/`ifndef/`elsif. @@ -276,26 +279,26 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, int macro_id = GetMacroIDOfConditional(current_node); bool negated = (current_node->token_enum() == PP_ifndef); // Checks if this macro is already assumed to be defined/undefined. - if (assumed.test(macro_id)) { + if (current_variant_.assumed.test(macro_id)) { bool assume_condition_is_true = - (negated ^ current_macros_.test(macro_id)); + (negated ^ current_variant_.macros_mask.test(macro_id)); if (auto status = DepthFirstSearch( receiver, edges_[current_node][!assume_condition_is_true], - assumed); + wants_more); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails."; return status; } } else { - assumed.flip(macro_id); + current_variant_.assumed.flip(macro_id); // This macro wans't assumed before, then we can check both edges. // Assume the condition is true. if (negated) - current_macros_.reset(macro_id); + current_variant_.macros_mask.reset(macro_id); else - current_macros_.set(macro_id); + current_variant_.macros_mask.set(macro_id); if (auto status = - DepthFirstSearch(receiver, edges_[current_node][0], assumed); + DepthFirstSearch(receiver, edges_[current_node][0], wants_more); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails."; return status; @@ -303,22 +306,24 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, // Assume the condition is false. if (!negated) - current_macros_.reset(macro_id); + current_variant_.macros_mask.reset(macro_id); else - current_macros_.set(macro_id); + current_variant_.macros_mask.set(macro_id); if (auto status = - DepthFirstSearch(receiver, edges_[current_node][1], assumed); + DepthFirstSearch(receiver, edges_[current_node][1], wants_more); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails."; return status; } + // Undo the change to allow for backtracking. + current_variant_.assumed.flip(macro_id); } } else { // Do recursive search through every possible edge. // Expected to be only one edge in this case. for (auto next_node : edges_[current_node]) { if (auto status = - FlowTree::DepthFirstSearch(receiver, next_node, assumed); + FlowTree::DepthFirstSearch(receiver, next_node, wants_more); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails\n"; return status; @@ -328,8 +333,7 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, // If the current node is the last one, push the completed current_sequence_ // then it is ready to be sent. if (current_node == source_sequence_.end() - 1) { - receiver(current_sequence_, variants_counter_, true); - variants_counter_++; + wants_more &= receiver(current_variant_); } if (current_node->token_enum() != PP_Identifier && current_node->token_enum() != PP_ifndef && @@ -340,7 +344,7 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, current_node->token_enum() != PP_else && current_node->token_enum() != PP_endif) { // Remove tokens to back track into other variants. - current_sequence_.pop_back(); + current_variant_.sequence.pop_back(); } return absl::OkStatus(); } diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index 186698d4a..674358c22 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -26,14 +26,9 @@ #include "verilog/parser/verilog_token_enum.h" namespace verilog { -using TokenSequenceConstIterator = verible::TokenSequence::const_iterator; -// Receive a complete token sequence of one variant. -// 1st argument: generated variant. -// 2nd argument: number of generated variants (to decide if more is needed). -// 3rd argument: checks if the variant is ready to be sent. -using VariantReceiver = - std::function; +using BitSet = std::bitset<128>; +using TokenSequenceConstIterator = verible::TokenSequence::const_iterator; struct ConditionalBlock { TokenSequenceConstIterator ifdef_iterator; @@ -43,6 +38,24 @@ struct ConditionalBlock { TokenSequenceConstIterator endif_iterator; }; +struct Variant { + // Contains the token sequence of the variant. + verible::TokenSequence sequence; + + // The i-th bit in "macros_mask" is 1 when the macro (with ID = i) is assumed + // to be defined, o.w. it is assumed to be undefined. + BitSet macros_mask; + + // The i-th bit in "assumed" is 1 when the macro (with ID = i) is visited or + // assumed (either defined or not), o.w. it is not visited (its value doesn't + // affect this variant). + BitSet assumed; +}; + +// Receive a complete token sequence of one variant. +// variant_sequence: the generated variant token sequence. +using VariantReceiver = std::function; + // FlowTree class builds the control flow graph of a tokenized System-Verilog // source code. Furthermore, enabling doing the following queries on the graph: // 1) Generating all the possible variants (provided via a callback function). @@ -52,22 +65,22 @@ class FlowTree { explicit FlowTree(verible::TokenSequence source_sequence) : source_sequence_(std::move(source_sequence)){}; - // Constructs the control flow tree by adding the tree edges in edges_. - absl::Status GenerateControlFlowTree(); - // Generates all possible variants. absl::Status GenerateVariants(const VariantReceiver &receiver); private: + // Constructs the control flow tree by adding the tree edges in edges_. + absl::Status GenerateControlFlowTree(); + // Traveses the tree in a depth first manner. absl::Status DepthFirstSearch(const VariantReceiver &receiver, TokenSequenceConstIterator current_node, - std::bitset<128> assumed); + bool wants_more); // Checks if the iterator points to a conditonal directive (`ifdef/ifndef...). static bool IsConditional(TokenSequenceConstIterator iterator); - // + // Adds all edges withing a conditional block. absl::Status AddBlockEdges(const ConditionalBlock &block); // The tree edges which defines the possible next childs of each token in @@ -91,22 +104,15 @@ class FlowTree { // The original source code lexed token seqouence. const verible::TokenSequence source_sequence_; - // The variant's token sequence currrently being built by DepthFirstSearch. - verible::TokenSequence current_sequence_; - - // Macros assumed in the variant that is currently begin built. - // TODO(karimtera): Is it better to use dymanic_bitset? - std::bitset<128> current_macros_; + // Current variant being generated by DepthFirstSearch. + Variant current_variant_; // Mapping each conditional macro to an integer ID, // to use it later as a bit offset. std::map conditional_macro_id_; // Number of macros appeared in `ifdef/`ifndef/`elsif. - int conditional_macros_counter = 0; - - // Number of vairants generated so far. - int variants_counter_ = 0; + int conditional_macros_counter_ = 0; }; } // namespace verilog diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 43f27d025..5d848fde8 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -34,18 +34,15 @@ using verible::SubcommandArgsRange; -bool PrintGeneratedVariant(const verible::TokenSequence& recieved_variant, - const int variants_counter, - const bool is_variant_complete) { - if (!is_variant_complete) { - // Only accepts 2 variants at most. (to test the functionality). - // TODO(karimtera): How many variants are practical? - return variants_counter < 20; - } - std::cout << "Variant number " << variants_counter + 1 << ":\n"; - for (auto token : recieved_variant) std::cout << token << '\n'; +bool PrintGeneratedVariant(const verilog::Variant& variant) { + static int variants_counter = 1; + std::cout << "Variant number " << variants_counter << ":\n"; + for (auto token : variant.sequence) std::cout << token << '\n'; puts(""); - return true; + variants_counter++; + // Only accepts 20 variants at most. (to test the functionality). + // TODO(karimtera): How many variants are practical? + return variants_counter < 20; } static absl::Status StripComments(const SubcommandArgsRange& args, @@ -159,12 +156,7 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, // Control flow tree constructing. verilog::FlowTree control_flow_tree(lexed_sequence); - auto status = control_flow_tree.GenerateControlFlowTree(); - if (!status.ok()) { - std::cerr << "ERROR:: couldn't generate the control flow tree.\n"; - return status; - } - status = control_flow_tree.GenerateVariants(PrintGeneratedVariant); + auto status = control_flow_tree.GenerateVariants(PrintGeneratedVariant); if (!status.ok()) { std::cerr << "ERROR:: couldn't generate variants.\n"; return status; From 1eaa85071869ec4bce17d97adc3e07bc1e7d4faa Mon Sep 17 00:00:00 2001 From: karimtera Date: Thu, 18 Aug 2022 02:49:55 +0200 Subject: [PATCH 11/17] Fixing clang-tidy issue in FlowTree. --- verilog/analysis/flow_tree.cc | 29 ++++++++++++----------------- verilog/analysis/flow_tree.h | 7 +++++-- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index f2be1dc30..bf57731e6 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -173,7 +173,7 @@ absl::Status FlowTree::GenerateVariants(const VariantReceiver &receiver) { return absl::InvalidArgumentError( "Error: creating the control flow graph has failed."); } - return DepthFirstSearch(receiver, source_sequence_.begin(), 1); + return DepthFirstSearch(receiver, source_sequence_.begin()); } // Constructs the control flow tree, which determines the edge from each node @@ -253,14 +253,13 @@ absl::Status FlowTree::GenerateControlFlowTree() { } // Traveses the control flow tree in a depth first manner, appending the visited -// tokens to current_sequence_, then provide the completed variant to the user +// tokens to current_variant_, then provide the completed variant to the user // using a callback function (VariantReceiver). -absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, - TokenSequenceConstIterator current_node, - bool wants_more) { - if (!wants_more) return absl::OkStatus(); +absl::Status FlowTree::DepthFirstSearch( + const VariantReceiver &receiver, TokenSequenceConstIterator current_node) { + if (!wants_more_) return absl::OkStatus(); - // Skips directives so that current_sequence_ doesn't contain any. + // Skips directives so that current_variant_ doesn't contain any. if (current_node->token_enum() != PP_Identifier && current_node->token_enum() != PP_ifndef && current_node->token_enum() != PP_ifdef && @@ -283,8 +282,7 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, bool assume_condition_is_true = (negated ^ current_variant_.macros_mask.test(macro_id)); if (auto status = DepthFirstSearch( - receiver, edges_[current_node][!assume_condition_is_true], - wants_more); + receiver, edges_[current_node][!assume_condition_is_true]); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails."; return status; @@ -297,8 +295,7 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, current_variant_.macros_mask.reset(macro_id); else current_variant_.macros_mask.set(macro_id); - if (auto status = - DepthFirstSearch(receiver, edges_[current_node][0], wants_more); + if (auto status = DepthFirstSearch(receiver, edges_[current_node][0]); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails."; return status; @@ -309,8 +306,7 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, current_variant_.macros_mask.reset(macro_id); else current_variant_.macros_mask.set(macro_id); - if (auto status = - DepthFirstSearch(receiver, edges_[current_node][1], wants_more); + if (auto status = DepthFirstSearch(receiver, edges_[current_node][1]); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails."; return status; @@ -322,18 +318,17 @@ absl::Status FlowTree::DepthFirstSearch(const VariantReceiver &receiver, // Do recursive search through every possible edge. // Expected to be only one edge in this case. for (auto next_node : edges_[current_node]) { - if (auto status = - FlowTree::DepthFirstSearch(receiver, next_node, wants_more); + if (auto status = FlowTree::DepthFirstSearch(receiver, next_node); !status.ok()) { std::cerr << "ERROR: DepthFirstSearch fails\n"; return status; } } } - // If the current node is the last one, push the completed current_sequence_ + // If the current node is the last one, push the completed current_variant_ // then it is ready to be sent. if (current_node == source_sequence_.end() - 1) { - wants_more &= receiver(current_variant_); + wants_more_ &= receiver(current_variant_); } if (current_node->token_enum() != PP_Identifier && current_node->token_enum() != PP_ifndef && diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index 674358c22..c85cc2e0b 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -74,8 +74,7 @@ class FlowTree { // Traveses the tree in a depth first manner. absl::Status DepthFirstSearch(const VariantReceiver &receiver, - TokenSequenceConstIterator current_node, - bool wants_more); + TokenSequenceConstIterator current_node); // Checks if the iterator points to a conditonal directive (`ifdef/ifndef...). static bool IsConditional(TokenSequenceConstIterator iterator); @@ -107,6 +106,10 @@ class FlowTree { // Current variant being generated by DepthFirstSearch. Variant current_variant_; + // A flag that determines if the VariantReceiver returned 'false'. + // By default: it assumes VariantReceiver wants more variants. + bool wants_more_ = 1; + // Mapping each conditional macro to an integer ID, // to use it later as a bit offset. std::map conditional_macro_id_; From 7c70b69a7823552fa6c4d0d07a00856654bb2ac1 Mon Sep 17 00:00:00 2001 From: karimtera Date: Wed, 24 Aug 2022 14:32:26 +0200 Subject: [PATCH 12/17] Adding a unit test for FlowTree class. --- verilog/analysis/BUILD | 16 ++ verilog/analysis/flow_tree.cc | 23 ++- verilog/analysis/flow_tree.h | 70 ++++--- verilog/analysis/flow_tree_test.cc | 185 ++++++++++++++++++ .../preprocessor/verilog_preprocessor.cc | 6 +- 5 files changed, 262 insertions(+), 38 deletions(-) create mode 100644 verilog/analysis/flow_tree_test.cc diff --git a/verilog/analysis/BUILD b/verilog/analysis/BUILD index 76ed6722b..6bfc08e3f 100644 --- a/verilog/analysis/BUILD +++ b/verilog/analysis/BUILD @@ -86,6 +86,22 @@ cc_library( ], ) +cc_test( + name = "flow_tree_test", + srcs = ["flow_tree_test.cc"], + deps = [ + ":flow_tree", + "//common/util:logging", + "//common/lexer:token_stream_adapter", + "//verilog/parser:verilog_lexer", + "//verilog/parser:verilog_token_enum", + "@com_google_absl//absl/memory", + "@com_google_absl//absl/strings", + "@com_google_googletest//:gtest_main", + ], +) + + cc_library( name = "lint_rule_registry", srcs = ["lint_rule_registry.cc"], diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index bf57731e6..6f2f4fec1 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -170,8 +170,7 @@ int FlowTree::GetMacroIDOfConditional( absl::Status FlowTree::GenerateVariants(const VariantReceiver &receiver) { auto status = GenerateControlFlowTree(); if (!status.ok()) { - return absl::InvalidArgumentError( - "Error: creating the control flow graph has failed."); + return status; } return DepthFirstSearch(receiver, source_sequence_.begin()); } @@ -200,7 +199,7 @@ absl::Status FlowTree::GenerateControlFlowTree() { auto status = AddMacroOfConditionalToMap(iter); if (!status.ok()) { return absl::InvalidArgumentError( - "Error couldn't give a macro an ID."); + "ERROR: couldn't give a macro an ID."); } break; } @@ -210,24 +209,33 @@ absl::Status FlowTree::GenerateControlFlowTree() { auto status = AddMacroOfConditionalToMap(iter); if (!status.ok()) { return absl::InvalidArgumentError( - "Error couldn't give a macro an ID."); + "ERROR: couldn't give a macro an ID."); } break; } case PP_elsif: { + if (if_blocks_.empty()) { + return absl::InvalidArgumentError("ERROR: Unmatched `elsif."); + } if_blocks_.back().elsif_iterators.push_back(iter); auto status = AddMacroOfConditionalToMap(iter); if (!status.ok()) { return absl::InvalidArgumentError( - "Error couldn't give a macro an ID."); + "ERROR: couldn't give a macro an ID."); } break; } case PP_else: { + if (if_blocks_.empty()) { + return absl::InvalidArgumentError("ERROR: Unmatched `else."); + } if_blocks_.back().else_iterator = iter; break; } case PP_endif: { + if (if_blocks_.empty()) { + return absl::InvalidArgumentError("ERROR: Unmatched `endif."); + } if_blocks_.back().endif_iterator = iter; auto status = AddBlockEdges(if_blocks_.back()); if (!status.ok()) return status; @@ -249,6 +257,11 @@ absl::Status FlowTree::GenerateControlFlowTree() { } } + // Checks for uncompleted conditionals. + if (!if_blocks_.empty()) + return absl::InvalidArgumentError( + "ERROR: Uncompleted conditional is found."); + return absl::OkStatus(); } diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index c85cc2e0b..2b30a6f80 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -17,7 +17,6 @@ #include #include -#include #include #include @@ -27,41 +26,52 @@ namespace verilog { -using BitSet = std::bitset<128>; -using TokenSequenceConstIterator = verible::TokenSequence::const_iterator; - -struct ConditionalBlock { - TokenSequenceConstIterator ifdef_iterator; - TokenSequenceConstIterator ifndef_iterator; - std::vector elsif_iterators; - TokenSequenceConstIterator else_iterator; - TokenSequenceConstIterator endif_iterator; -}; - -struct Variant { - // Contains the token sequence of the variant. - verible::TokenSequence sequence; - - // The i-th bit in "macros_mask" is 1 when the macro (with ID = i) is assumed - // to be defined, o.w. it is assumed to be undefined. - BitSet macros_mask; - - // The i-th bit in "assumed" is 1 when the macro (with ID = i) is visited or - // assumed (either defined or not), o.w. it is not visited (its value doesn't - // affect this variant). - BitSet assumed; -}; - -// Receive a complete token sequence of one variant. -// variant_sequence: the generated variant token sequence. -using VariantReceiver = std::function; - // FlowTree class builds the control flow graph of a tokenized System-Verilog // source code. Furthermore, enabling doing the following queries on the graph: // 1) Generating all the possible variants (provided via a callback function). // 2) TODO(karimtera): uniquely identify a variant with a bitset. class FlowTree { public: + using BitSet = std::bitset<128>; + using TokenSequenceConstIterator = verible::TokenSequence::const_iterator; + + struct ConditionalBlock { + TokenSequenceConstIterator ifdef_iterator; + TokenSequenceConstIterator ifndef_iterator; + std::vector elsif_iterators; + TokenSequenceConstIterator else_iterator; + TokenSequenceConstIterator endif_iterator; + }; + + struct Variant { + // Contains the token sequence of the variant. + verible::TokenSequence sequence; + + // The i-th bit in "macros_mask" is 1 when the macro (with ID = i) is + // assumed to be defined, o.w. it is assumed to be undefined. + BitSet macros_mask; + + // The i-th bit in "assumed" is 1 when the macro (with ID = i) is visited or + // assumed (either defined or not), o.w. it is not visited (its value + // doesn't affect this variant). + // + // e.g.: + // `ifdef A + // `ifdef B + // ... + // `endif + // `endif + // + // Consider the variant in which A is undefined, + // we notice that B doesn't affect the variant. + // Then the bit corresponding to B in "assumed" is 0. + BitSet assumed; + }; + + // Receive a complete token sequence of one variant. + // variant_sequence: the generated variant token sequence. + using VariantReceiver = std::function; + explicit FlowTree(verible::TokenSequence source_sequence) : source_sequence_(std::move(source_sequence)){}; diff --git a/verilog/analysis/flow_tree_test.cc b/verilog/analysis/flow_tree_test.cc new file mode 100644 index 000000000..f3c34c7b4 --- /dev/null +++ b/verilog/analysis/flow_tree_test.cc @@ -0,0 +1,185 @@ +// Copyright 2017-2022 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "verilog/analysis/flow_tree.h" + +#include + +#include "absl/status/status.h" +#include "absl/strings/string_view.h" +#include "common/lexer/token_stream_adapter.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "verilog/parser/verilog_lexer.h" +#include "verilog/parser/verilog_token_enum.h" + +namespace verilog { +namespace { + +using testing::StartsWith; +using VariantReceiver = std::function; + +// Global variables for testing, 'VariantReceiver's updates them. +std::vector g_variants; + +// Resets the global variants vector (to be used before any test). +void ResetGlobalTestingVariables() { g_variants.clear(); } + +// Lexes a SystemVerilog source code, and returns a TokenSequence. +verible::TokenSequence LexToSequence(absl::string_view source_contents) { + verible::TokenSequence lexed_sequence; + VerilogLexer lexer(source_contents); + for (lexer.DoNextToken(); !lexer.GetLastToken().isEOF(); + lexer.DoNextToken()) { + if (verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken())) { + lexed_sequence.push_back(lexer.GetLastToken()); + } + } + return lexed_sequence; +} + +// A VariantReceiver function that add variants into g_variants. +bool GetVariant(const FlowTree::Variant& variant) { + g_variants.push_back(variant); + return true; +} + +TEST(FlowTree, MultipleConditionalsSameMacro) { + const absl::string_view test_case = + R"( + `ifdef A + A_TRUE + `else + A_FALSE + `endif + + `ifdef A + A_TRUE + `else + A_FALSE + `endif + + `ifndef A + A_FALSE + `else + A_TRUE + `endif)"; + + FlowTree tree_test(LexToSequence(test_case)); + ResetGlobalTestingVariables(); + auto status = tree_test.GenerateVariants(GetVariant); + EXPECT_TRUE(status.ok()); + EXPECT_EQ(g_variants.size(), 2); + for (const auto& variant : g_variants[0].sequence) { + EXPECT_EQ(variant.text(), "A_TRUE"); + } + for (const auto& variant : g_variants[1].sequence) { + EXPECT_EQ(variant.text(), "A_FALSE"); + } +} + +TEST(FlowTree, UnmatchedElses) { + const absl::string_view test_cases[] = { + R"( + `elsif A + A_TRUE + `endif + )", + R"( + `ifdef A + A_TRUE + `endif + `elsif B + B_TRUE + `endif + )", + R"( + `else + A_TRUE + `endif + )", + R"( + `ifdef A + `elsif B + B_TRUE + `endif + `endif + )", + R"( + `endif + )"}; + + for (auto test : test_cases) { + FlowTree tree_test(LexToSequence(test)); + ResetGlobalTestingVariables(); + auto status = tree_test.GenerateVariants(GetVariant); + EXPECT_FALSE(status.ok()); + EXPECT_THAT(status.message(), StartsWith("ERROR: Unmatched")); + } +} + +TEST(FlowTree, UncompletedConditionals) { + const absl::string_view test_cases[] = { + R"( + `ifdef A + A_TRUE + `else + A_FALSE + )", + R"( + `ifdef A + A_TRUE + `ifndef B + B_FALSE + `else + B_TRUE + `endif + )"}; + + for (auto test : test_cases) { + FlowTree tree_test(LexToSequence(test)); + ResetGlobalTestingVariables(); + auto status = tree_test.GenerateVariants(GetVariant); + EXPECT_FALSE(status.ok()); + EXPECT_THAT(status.message(), StartsWith("ERROR: Uncompleted")); + } +} + +TEST(FlowTree, NestedConditionals) { + const absl::string_view test_cases[] = { + R"( + `ifdef A + `ifdef B + A_B + `else + A_nB + `endif + `else + nA_B_or_nA_nB + `endif)"}; + + for (auto test : test_cases) { + FlowTree tree_test(LexToSequence(test)); + ResetGlobalTestingVariables(); + auto status = tree_test.GenerateVariants(GetVariant); + EXPECT_TRUE(status.ok()); + EXPECT_EQ(g_variants.size(), 3); + for (const auto& variant : g_variants) { + EXPECT_EQ(variant.sequence.size(), 1); + } + } +} + +} // namespace +} // namespace verilog diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 5d848fde8..62deb728c 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -34,7 +34,7 @@ using verible::SubcommandArgsRange; -bool PrintGeneratedVariant(const verilog::Variant& variant) { +bool PrintGeneratedVariant(const verilog::FlowTree::Variant& variant) { static int variants_counter = 1; std::cout << "Variant number " << variants_counter << ":\n"; for (auto token : variant.sequence) std::cout << token << '\n'; @@ -149,8 +149,9 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, // For now we will store the syntax tree tokens only, ignoring all the // white-space characters. however that should be stored to output the // source code just like it was. - if (verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken())) + if (verilog::VerilogLexer::KeepSyntaxTreeTokens(lexer.GetLastToken())) { lexed_sequence.push_back(lexer.GetLastToken()); + } } // Control flow tree constructing. @@ -158,7 +159,6 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, auto status = control_flow_tree.GenerateVariants(PrintGeneratedVariant); if (!status.ok()) { - std::cerr << "ERROR:: couldn't generate variants.\n"; return status; } From 4006cf8e4bbe72841ff9ae782a778d49157c4933 Mon Sep 17 00:00:00 2001 From: karimtera Date: Tue, 30 Aug 2022 05:47:32 +0200 Subject: [PATCH 13/17] FlowTree class: - Added API GetUsedMacros that returns all the used macros in conditionals. - Changed "assumed" bitset name to "visited". - Added description to "ConditionalBlock". - Added more testing using closures. Preprocessor tool: - Kept the subcommand part. - Available subcommands for now "strip-comments", "multiple-cu", and "generate-variants". --- verilog/analysis/flow_tree.cc | 94 +++++++++---------- verilog/analysis/flow_tree.h | 54 ++++++++--- verilog/analysis/flow_tree_test.cc | 90 +++++++++++------- .../preprocessor/verilog_preprocessor.cc | 93 ++++++++++-------- .../preprocessor/verilog_preprocessor_test.sh | 77 ++------------- 5 files changed, 203 insertions(+), 205 deletions(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index 6f2f4fec1..3007b2232 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -1,17 +1,16 @@ // Copyright 2017-2022 The Verible Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance wedge_from_iteratorh the -// License. You may obtain a copy of the License at +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at // // http://www.apache.org/licenses/LICENSE-2.0 // -// Unless required by applicable law or agreed to in wredge_from_iteratoring, +// Unless required by applicable law or agreed to in writing, // software distributed under the License is distributed on an "AS IS" BASIS, -// Wedge_from_iteratorHOUT WARRANTIES OR CONDedge_from_iteratorIONS OF ANY KIND, -// eedge_from_iteratorher express or implied. See the License for the specific -// language governing permissions and limedge_from_iteratorations under the -// License. +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. #include "verilog/analysis/flow_tree.h" @@ -31,50 +30,47 @@ namespace verilog { // Such that the first edge represents the condition being true, // and the second edge represents the condition being false. absl::Status FlowTree::AddBlockEdges(const ConditionalBlock &block) { - bool contains_elsif = !block.elsif_iterators.empty(); - bool contains_else = block.else_iterator != source_sequence_.end(); + bool contains_elsif = !block.elsif_locations.empty(); + bool contains_else = block.else_location != source_sequence_.end(); // Handling `ifdef/ifndef. - auto ifdef_or_ifndef = - (block.ifdef_iterator == source_sequence_.end() ? block.ifndef_iterator - : block.ifdef_iterator); // Assuming the condition is true. - edges_[ifdef_or_ifndef].push_back(ifdef_or_ifndef + 1); + edges_[block.if_location].push_back(block.if_location + 1); // Assuming the condition is false. // Checking if there is an `elsif. if (contains_elsif) { // Add edge to the first `elsif in the block. - edges_[ifdef_or_ifndef].push_back(block.elsif_iterators[0]); + edges_[block.if_location].push_back(block.elsif_locations[0]); } else if (contains_else) { // Checking if there is an `else. - edges_[ifdef_or_ifndef].push_back(block.else_iterator); + edges_[block.if_location].push_back(block.else_location); } else { // `endif exists. - edges_[ifdef_or_ifndef].push_back(block.endif_iterator); + edges_[block.if_location].push_back(block.endif_location); } // Handling `elsif. if (contains_elsif) { - for (auto iter = block.elsif_iterators.begin(); - iter != block.elsif_iterators.end(); iter++) { + for (auto iter = block.elsif_locations.begin(); + iter != block.elsif_locations.end(); iter++) { // Assuming the condition is true. edges_[*iter].push_back((*iter) + 1); // Assuming the condition is false. - if (iter + 1 != block.elsif_iterators.end()) + if (iter + 1 != block.elsif_locations.end()) edges_[*iter].push_back(*(iter + 1)); else if (contains_else) - edges_[*iter].push_back(block.else_iterator); + edges_[*iter].push_back(block.else_location); else - edges_[*iter].push_back(block.endif_iterator); + edges_[*iter].push_back(block.endif_location); } } // Handling `else. if (contains_else) { - edges_[block.else_iterator].push_back(block.else_iterator + 1); + edges_[block.else_location].push_back(block.else_location + 1); } // For edges that are generated assuming the conditons are true, @@ -88,21 +84,21 @@ absl::Status FlowTree::AddBlockEdges(const ConditionalBlock &block) { // // `endif // Edge to be added: from to `endif. - edges_[block.endif_iterator - 1].push_back(block.endif_iterator); + edges_[block.endif_location - 1].push_back(block.endif_location); if (contains_elsif) { - for (auto iter : block.elsif_iterators) - edges_[iter - 1].push_back(block.endif_iterator); + for (auto iter : block.elsif_locations) + edges_[iter - 1].push_back(block.endif_location); } else if (contains_else) { - edges_[block.else_iterator - 1].push_back(block.endif_iterator); + edges_[block.else_location - 1].push_back(block.endif_location); } // Connecting `endif to the next token directly (if not EOF). - auto next_iter = block.endif_iterator + 1; + auto next_iter = block.endif_location + 1; if (next_iter != source_sequence_.end() && next_iter->token_enum() != PP_else && next_iter->token_enum() != PP_elsif && next_iter->token_enum() != PP_endif) { - edges_[block.endif_iterator].push_back(next_iter); + edges_[block.endif_location].push_back(next_iter); } return absl::OkStatus(); @@ -132,9 +128,9 @@ absl::Status FlowTree::MacroFollows( return absl::OkStatus(); } -// Adds a conditional macro to conditional_macro_id_ if not added before, -// And gives it a new ID. -absl::Status FlowTree::AddMacroOfConditionalToMap( +// Adds a conditional macro to conditional_macros_ if not added before, +// And gives it a new ID, then saves the ID in conditional_macro_id_ map. +absl::Status FlowTree::AddMacroOfConditional( TokenSequenceConstIterator conditional_iterator) { auto status = MacroFollows(conditional_iterator); if (!status.ok()) { @@ -146,6 +142,7 @@ absl::Status FlowTree::AddMacroOfConditionalToMap( if (conditional_macro_id_.find(macro_identifier) == conditional_macro_id_.end()) { conditional_macro_id_[macro_identifier] = conditional_macros_counter_; + conditional_macros_.push_back(macro_iterator); conditional_macros_counter_++; } return absl::OkStatus(); @@ -182,10 +179,9 @@ absl::Status FlowTree::GenerateControlFlowTree() { // Adding edges for if blocks. int current_token_enum = 0; ConditionalBlock empty_block; - empty_block.ifdef_iterator = source_sequence_.end(); - empty_block.ifndef_iterator = source_sequence_.end(); - empty_block.else_iterator = source_sequence_.end(); - empty_block.endif_iterator = source_sequence_.end(); + empty_block.if_location = source_sequence_.end(); + empty_block.else_location = source_sequence_.end(); + empty_block.endif_location = source_sequence_.end(); for (TokenSequenceConstIterator iter = source_sequence_.begin(); iter != source_sequence_.end(); iter++) { @@ -195,8 +191,9 @@ absl::Status FlowTree::GenerateControlFlowTree() { switch (current_token_enum) { case PP_ifdef: { if_blocks_.push_back(empty_block); - if_blocks_.back().ifdef_iterator = iter; - auto status = AddMacroOfConditionalToMap(iter); + if_blocks_.back().if_location = iter; + if_blocks_.back().positive_condition = 1; + auto status = AddMacroOfConditional(iter); if (!status.ok()) { return absl::InvalidArgumentError( "ERROR: couldn't give a macro an ID."); @@ -205,8 +202,9 @@ absl::Status FlowTree::GenerateControlFlowTree() { } case PP_ifndef: { if_blocks_.push_back(empty_block); - if_blocks_.back().ifndef_iterator = iter; - auto status = AddMacroOfConditionalToMap(iter); + if_blocks_.back().if_location = iter; + if_blocks_.back().positive_condition = 0; + auto status = AddMacroOfConditional(iter); if (!status.ok()) { return absl::InvalidArgumentError( "ERROR: couldn't give a macro an ID."); @@ -217,8 +215,8 @@ absl::Status FlowTree::GenerateControlFlowTree() { if (if_blocks_.empty()) { return absl::InvalidArgumentError("ERROR: Unmatched `elsif."); } - if_blocks_.back().elsif_iterators.push_back(iter); - auto status = AddMacroOfConditionalToMap(iter); + if_blocks_.back().elsif_locations.push_back(iter); + auto status = AddMacroOfConditional(iter); if (!status.ok()) { return absl::InvalidArgumentError( "ERROR: couldn't give a macro an ID."); @@ -229,14 +227,14 @@ absl::Status FlowTree::GenerateControlFlowTree() { if (if_blocks_.empty()) { return absl::InvalidArgumentError("ERROR: Unmatched `else."); } - if_blocks_.back().else_iterator = iter; + if_blocks_.back().else_location = iter; break; } case PP_endif: { if (if_blocks_.empty()) { return absl::InvalidArgumentError("ERROR: Unmatched `endif."); } - if_blocks_.back().endif_iterator = iter; + if_blocks_.back().endif_location = iter; auto status = AddBlockEdges(if_blocks_.back()); if (!status.ok()) return status; // TODO(karimtera): add an error message. @@ -290,8 +288,8 @@ absl::Status FlowTree::DepthFirstSearch( current_node->token_enum() == PP_elsif) { int macro_id = GetMacroIDOfConditional(current_node); bool negated = (current_node->token_enum() == PP_ifndef); - // Checks if this macro is already assumed to be defined/undefined. - if (current_variant_.assumed.test(macro_id)) { + // Checks if this macro is already visited (either defined/undefined). + if (current_variant_.visited.test(macro_id)) { bool assume_condition_is_true = (negated ^ current_variant_.macros_mask.test(macro_id)); if (auto status = DepthFirstSearch( @@ -301,8 +299,8 @@ absl::Status FlowTree::DepthFirstSearch( return status; } } else { - current_variant_.assumed.flip(macro_id); - // This macro wans't assumed before, then we can check both edges. + current_variant_.visited.flip(macro_id); + // This macro wans't visited before, then we can check both edges. // Assume the condition is true. if (negated) current_variant_.macros_mask.reset(macro_id); @@ -325,7 +323,7 @@ absl::Status FlowTree::DepthFirstSearch( return status; } // Undo the change to allow for backtracking. - current_variant_.assumed.flip(macro_id); + current_variant_.visited.flip(macro_id); } } else { // Do recursive search through every possible edge. diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index 2b30a6f80..038a01785 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -28,19 +28,33 @@ namespace verilog { // FlowTree class builds the control flow graph of a tokenized System-Verilog // source code. Furthermore, enabling doing the following queries on the graph: -// 1) Generating all the possible variants (provided via a callback function). -// 2) TODO(karimtera): uniquely identify a variant with a bitset. +// - Generating all the possible variants (provided via a callback function). class FlowTree { + private: + // 'kMaxDistinctMacros' shows the maximum number of distinct macros that can + // be considered in conditonal directives. + static constexpr int kMaxDistinctMacros = 128; + public: - using BitSet = std::bitset<128>; + using BitSet = std::bitset; using TokenSequenceConstIterator = verible::TokenSequence::const_iterator; + // "ConditionalBlock" saves locations of conditionals in a "TokenSequence". + // All locations should point inside this specific "TokenSequence". + // Since it is only used in conjunction with a "TokenSequence", + // It should be initialized to last location in this "TokenSequence", + // For example in "GenerateControlFlowTree" is initialized to + // 'source_sequence_.end()'. struct ConditionalBlock { - TokenSequenceConstIterator ifdef_iterator; - TokenSequenceConstIterator ifndef_iterator; - std::vector elsif_iterators; - TokenSequenceConstIterator else_iterator; - TokenSequenceConstIterator endif_iterator; + // "if_location" points to `ifdef or `ifndef. + TokenSequenceConstIterator if_location; + + // When "positive_condition" equals 1, then "if_location" points to `ifdef, + // Otherwise it points to `ifndef. + bool positive_condition; + std::vector elsif_locations; + TokenSequenceConstIterator else_location; + TokenSequenceConstIterator endif_location; }; struct Variant { @@ -48,11 +62,11 @@ class FlowTree { verible::TokenSequence sequence; // The i-th bit in "macros_mask" is 1 when the macro (with ID = i) is - // assumed to be defined, o.w. it is assumed to be undefined. + // assumed to be defined, otherwise it is assumed to be undefined. BitSet macros_mask; - // The i-th bit in "assumed" is 1 when the macro (with ID = i) is visited or - // assumed (either defined or not), o.w. it is not visited (its value + // The i-th bit in "visited" is 1 when the macro (with ID = i) is visited or + // assumed (either defined or not), otherwise it is not visited (its value // doesn't affect this variant). // // e.g.: @@ -64,8 +78,8 @@ class FlowTree { // // Consider the variant in which A is undefined, // we notice that B doesn't affect the variant. - // Then the bit corresponding to B in "assumed" is 0. - BitSet assumed; + // Then the bit corresponding to B in "visited" is 0. + BitSet visited; }; // Receive a complete token sequence of one variant. @@ -78,6 +92,12 @@ class FlowTree { // Generates all possible variants. absl::Status GenerateVariants(const VariantReceiver &receiver); + // Returns all the used macros in conditionals, ordered with the same ID as + // used in BitSets. + const std::vector &GetUsedMacros() { + return conditional_macros_; + } + private: // Constructs the control flow tree by adding the tree edges in edges_. absl::Status GenerateControlFlowTree(); @@ -101,8 +121,9 @@ class FlowTree { static absl::Status MacroFollows( TokenSequenceConstIterator conditional_iterator); - // Adds macro to conditional_macro_id_ map. - absl::Status AddMacroOfConditionalToMap( + // Adds macro to conditional_macros_ vector, and save its ID in + // conditional_macro_id_ map. + absl::Status AddMacroOfConditional( TokenSequenceConstIterator conditional_iterator); int GetMacroIDOfConditional(TokenSequenceConstIterator conditional_iterator); @@ -124,6 +145,9 @@ class FlowTree { // to use it later as a bit offset. std::map conditional_macro_id_; + // A vector containing all the macros used placed by their given ID. + std::vector conditional_macros_; + // Number of macros appeared in `ifdef/`ifndef/`elsif. int conditional_macros_counter_ = 0; }; diff --git a/verilog/analysis/flow_tree_test.cc b/verilog/analysis/flow_tree_test.cc index f3c34c7b4..0eb805ec5 100644 --- a/verilog/analysis/flow_tree_test.cc +++ b/verilog/analysis/flow_tree_test.cc @@ -28,13 +28,6 @@ namespace verilog { namespace { using testing::StartsWith; -using VariantReceiver = std::function; - -// Global variables for testing, 'VariantReceiver's updates them. -std::vector g_variants; - -// Resets the global variants vector (to be used before any test). -void ResetGlobalTestingVariables() { g_variants.clear(); } // Lexes a SystemVerilog source code, and returns a TokenSequence. verible::TokenSequence LexToSequence(absl::string_view source_contents) { @@ -49,44 +42,54 @@ verible::TokenSequence LexToSequence(absl::string_view source_contents) { return lexed_sequence; } -// A VariantReceiver function that add variants into g_variants. -bool GetVariant(const FlowTree::Variant& variant) { - g_variants.push_back(variant); - return true; -} - TEST(FlowTree, MultipleConditionalsSameMacro) { const absl::string_view test_case = R"( `ifdef A - A_TRUE + A_TRUE_1 `else - A_FALSE + A_FALSE_1 `endif `ifdef A - A_TRUE + A_TRUE_2 `else - A_FALSE + A_FALSE_2 `endif `ifndef A - A_FALSE + A_FALSE_3 `else - A_TRUE + A_TRUE_3 `endif)"; FlowTree tree_test(LexToSequence(test_case)); - ResetGlobalTestingVariables(); - auto status = tree_test.GenerateVariants(GetVariant); + std::vector variants; + auto status = + tree_test.GenerateVariants([&variants](const FlowTree::Variant& variant) { + variants.push_back(variant); + return true; + }); EXPECT_TRUE(status.ok()); - EXPECT_EQ(g_variants.size(), 2); - for (const auto& variant : g_variants[0].sequence) { - EXPECT_EQ(variant.text(), "A_TRUE"); - } - for (const auto& variant : g_variants[1].sequence) { - EXPECT_EQ(variant.text(), "A_FALSE"); - } + EXPECT_EQ(variants.size(), 2); + + const auto& used_macros = tree_test.GetUsedMacros(); + EXPECT_EQ(used_macros.size(), 1); + EXPECT_THAT(used_macros[0]->text(), "A"); + + // First variant: A is defined. + EXPECT_TRUE(variants[0].macros_mask.test(0)); + EXPECT_TRUE(variants[0].visited.test(0)); + EXPECT_THAT(variants[0].sequence[0].text(), "A_TRUE_1"); + EXPECT_THAT(variants[0].sequence[1].text(), "A_TRUE_2"); + EXPECT_THAT(variants[0].sequence[2].text(), "A_TRUE_3"); + + // Second variant: A is undefined. + EXPECT_FALSE(variants[1].macros_mask.test(0)); + EXPECT_TRUE(variants[1].visited.test(0)); + EXPECT_THAT(variants[1].sequence[0].text(), "A_FALSE_1"); + EXPECT_THAT(variants[1].sequence[1].text(), "A_FALSE_2"); + EXPECT_THAT(variants[1].sequence[2].text(), "A_FALSE_3"); } TEST(FlowTree, UnmatchedElses) { @@ -122,8 +125,8 @@ TEST(FlowTree, UnmatchedElses) { for (auto test : test_cases) { FlowTree tree_test(LexToSequence(test)); - ResetGlobalTestingVariables(); - auto status = tree_test.GenerateVariants(GetVariant); + auto status = tree_test.GenerateVariants( + [](const FlowTree::Variant& variant) { return true; }); EXPECT_FALSE(status.ok()); EXPECT_THAT(status.message(), StartsWith("ERROR: Unmatched")); } @@ -149,8 +152,8 @@ TEST(FlowTree, UncompletedConditionals) { for (auto test : test_cases) { FlowTree tree_test(LexToSequence(test)); - ResetGlobalTestingVariables(); - auto status = tree_test.GenerateVariants(GetVariant); + auto status = tree_test.GenerateVariants( + [](const FlowTree::Variant& variant) { return true; }); EXPECT_FALSE(status.ok()); EXPECT_THAT(status.message(), StartsWith("ERROR: Uncompleted")); } @@ -171,13 +174,28 @@ TEST(FlowTree, NestedConditionals) { for (auto test : test_cases) { FlowTree tree_test(LexToSequence(test)); - ResetGlobalTestingVariables(); - auto status = tree_test.GenerateVariants(GetVariant); + std::vector variants; + auto status = tree_test.GenerateVariants( + [&variants](const FlowTree::Variant& variant) { + variants.push_back(variant); + return true; + }); EXPECT_TRUE(status.ok()); - EXPECT_EQ(g_variants.size(), 3); - for (const auto& variant : g_variants) { + EXPECT_EQ(variants.size(), 3); + for (const auto& variant : variants) { EXPECT_EQ(variant.sequence.size(), 1); + if (variant.macros_mask.test(0) == 0) { + // Check that if A is undefined, then B is not visited. + EXPECT_FALSE(variant.visited.test(1)); + } else { + // Check that if A is defined, then B is visited. + EXPECT_TRUE(variant.visited.test(1)); + } } + const auto& used_macros = tree_test.GetUsedMacros(); + EXPECT_EQ(used_macros.size(), 2); + EXPECT_THAT(used_macros[0]->text(), "A"); + EXPECT_THAT(used_macros[1]->text(), "B"); } } diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index 62deb728c..dd0136785 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -33,17 +33,9 @@ #include "verilog/transform/strip_comments.h" using verible::SubcommandArgsRange; +using verible::SubcommandEntry; -bool PrintGeneratedVariant(const verilog::FlowTree::Variant& variant) { - static int variants_counter = 1; - std::cout << "Variant number " << variants_counter << ":\n"; - for (auto token : variant.sequence) std::cout << token << '\n'; - puts(""); - variants_counter++; - // Only accepts 20 variants at most. (to test the functionality). - // TODO(karimtera): How many variants are practical? - return variants_counter < 20; -} +ABSL_FLAG(int, limit_variants, 20, "Maximum number of variants printed"); static absl::Status StripComments(const SubcommandArgsRange& args, std::istream&, std::ostream& outs, @@ -80,8 +72,8 @@ static absl::Status StripComments(const SubcommandArgsRange& args, return absl::OkStatus(); } -static absl::Status MultipleCU(const char* source_file, std::istream&, - std::ostream& outs, std::ostream&) { +static absl::Status PreprocessSingleFile(const char* source_file, std::istream&, + std::ostream& outs, std::ostream&) { std::string source_contents; if (auto status = verible::file::GetContents(source_file, &source_contents); !status.ok()) { @@ -132,8 +124,29 @@ static absl::Status MultipleCU(const char* source_file, std::istream&, return absl::OkStatus(); } -static absl::Status GenerateVariants(const char* source_file, std::istream&, - std::ostream& outs, std::ostream&) { +static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&, + std::ostream& outs, std::ostream&) { + if (args.empty()) { + return absl::InvalidArgumentError("Missing file arguments."); + } + for (const char* source_file : args) { + outs << source_file << ":\n"; + auto status = PreprocessSingleFile(source_file, std::cin, outs, std::cerr); + if (!status.ok()) return status; + outs << '\n'; + } + return absl::OkStatus(); +} + +static absl::Status GenerateVariants(const SubcommandArgsRange& args, + std::istream&, std::ostream& outs, + std::ostream&) { + const int limit_variants = absl::GetFlag(FLAGS_limit_variants); + if (args.size() > 1) { + return absl::InvalidArgumentError( + "ERROR: generate-variants only works on one file."); + } + const char* source_file = args[0]; std::string source_contents; if (auto status = verible::file::GetContents(source_file, &source_contents); !status.ok()) { @@ -156,8 +169,16 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, // Control flow tree constructing. verilog::FlowTree control_flow_tree(lexed_sequence); - - auto status = control_flow_tree.GenerateVariants(PrintGeneratedVariant); + int counter = 0; + auto status = control_flow_tree.GenerateVariants( + [limit_variants, &counter](const verilog::FlowTree::Variant& variant) { + if (counter == limit_variants) return false; + counter++; + std::cout << "Variant number " << counter << ":\n"; + for (auto token : variant.sequence) std::cout << token << '\n'; + puts(""); + return true; + }); if (!status.ok()) { return status; } @@ -166,45 +187,41 @@ static absl::Status GenerateVariants(const char* source_file, std::istream&, } static const std::pair kCommands[] = { - {"strip-comments", // - {&StripComments, // + {"strip-comments", + {&StripComments, R"(strip-comments file [replacement-char] - Inputs: 'file' is a Verilog or SystemVerilog source file. Use '-' to read from stdin. - 'replacement-char' is a character to replace comments with. If not given, or given as a single space character, the comment contents and delimiters are replaced with spaces. If an empty string, the comment contents and delimiters are deleted. Newlines are not deleted. If a single character, the comment contents are replaced with the character. - Output: (stdout) Contents of original file with // and /**/ comments removed. )"}}, - - {"multiple-cu", // - {&MultipleCU, // - R"(multiple-cu file1 file2 ... - -Input: - 'file's are Verilog or SystemVerilog source files. - each one will be preprocessed in a separate compilation unit. + {"multiple-cu", + {&MultipleCU, + R"(multiple-cu file [more_files] +Inputs: + 'file' is a Verilog or SystemVerilog source file. + There can be multiple SystemVerilog source files. + Each one of them will be compiled in a separate compilation unit. Output: (stdout) - Contents of original file with compiler directives interpreted. + The preprocessed files content (same contents with directives interpreted). )"}}, - - {"generate-variants", // - {&GenerateVariants, // - R"(bypass-conditionals file - -Input: - 'file' is Verilog or SystemVerilog source file. + {"generate-variants", + {&GenerateVariants, + R"(genera-variants file [-limit_variants number] +Inputs: + 'file' is a Verilog or SystemVerilog source file. + '-limit_variants' flag limits variants to 'number' (20 by default). Output: (stdout) - Every possible source variants. + Generates every possible variant considering the conditional directives. )"}}, + }; int main(int argc, char* argv[]) { diff --git a/verilog/tools/preprocessor/verilog_preprocessor_test.sh b/verilog/tools/preprocessor/verilog_preprocessor_test.sh index c8feda753..7c1704be6 100755 --- a/verilog/tools/preprocessor/verilog_preprocessor_test.sh +++ b/verilog/tools/preprocessor/verilog_preprocessor_test.sh @@ -51,33 +51,7 @@ status="$?" } ################################################################################ -echo "=== Test -strip_comments: white out comments" - -cat > "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_OUTPUT_FILE" 2>&1 @@ -115,7 +89,7 @@ status="$?" exit 1 } -################################################################################ +################################################################################################################################################################ echo "=== Test strip-comments: white out comments" cat > "$MY_INPUT_FILE" < "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_OUTPUT_FILE" @@ -203,7 +150,6 @@ cat > "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < Date: Thu, 1 Sep 2022 13:58:52 +0200 Subject: [PATCH 14/17] Commit changes: - Moved ConditionalBlock to private section in FlowTree class. - Removed unwanted debug code from the preprocessor tool. - Fixed output streams issues in the preprocessor tool. --- verilog/analysis/flow_tree.h | 23 +- .../preprocessor/verilog_preprocessor.cc | 59 ++--- .../preprocessor/verilog_preprocessor_test.sh | 240 +++++++++++++++++- 3 files changed, 277 insertions(+), 45 deletions(-) diff --git a/verilog/analysis/flow_tree.h b/verilog/analysis/flow_tree.h index 038a01785..f9b0141f1 100644 --- a/verilog/analysis/flow_tree.h +++ b/verilog/analysis/flow_tree.h @@ -45,17 +45,6 @@ class FlowTree { // It should be initialized to last location in this "TokenSequence", // For example in "GenerateControlFlowTree" is initialized to // 'source_sequence_.end()'. - struct ConditionalBlock { - // "if_location" points to `ifdef or `ifndef. - TokenSequenceConstIterator if_location; - - // When "positive_condition" equals 1, then "if_location" points to `ifdef, - // Otherwise it points to `ifndef. - bool positive_condition; - std::vector elsif_locations; - TokenSequenceConstIterator else_location; - TokenSequenceConstIterator endif_location; - }; struct Variant { // Contains the token sequence of the variant. @@ -99,6 +88,18 @@ class FlowTree { } private: + struct ConditionalBlock { + // "if_location" points to `ifdef or `ifndef. + TokenSequenceConstIterator if_location; + + // When "positive_condition" equals 1, then "if_location" points to `ifdef, + // Otherwise it points to `ifndef. + bool positive_condition; + std::vector elsif_locations; + TokenSequenceConstIterator else_location; + TokenSequenceConstIterator endif_location; + }; + // Constructs the control flow tree by adding the tree edges in edges_. absl::Status GenerateControlFlowTree(); diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index dd0136785..d47b83e01 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -73,11 +73,12 @@ static absl::Status StripComments(const SubcommandArgsRange& args, } static absl::Status PreprocessSingleFile(const char* source_file, std::istream&, - std::ostream& outs, std::ostream&) { + std::ostream& outs, + std::ostream& message_stream) { std::string source_contents; if (auto status = verible::file::GetContents(source_file, &source_contents); !status.ok()) { - std::cerr << "ERROR: passed file can't be open\n."; + message_stream << source_file << status; return status; } verilog::VerilogPreprocess::Config config; @@ -101,36 +102,19 @@ static absl::Status PreprocessSingleFile(const char* source_file, std::istream&, preprocessor.ScanStream(lexed_streamview); auto& preprocessed_stream = preprocessed_data.preprocessed_token_stream; for (auto u : preprocessed_stream) outs << *u << '\n'; - // for debugging. for (auto& u : preprocessed_data.errors) outs << u.error_message << '\n'; - // parsing just as a trial - std::string post_preproc; - for (auto u : preprocessed_stream) post_preproc += std::string{u->text()}; - std::string source_view{post_preproc}; - verilog::VerilogAnalyzer analyzer(source_view, "file1", config); - auto analyze_status = analyzer.Analyze(); - /* const auto& mydata = analyzer.Data().Contents(); */ - /* outs< 1) { return absl::InvalidArgumentError( @@ -150,8 +134,8 @@ static absl::Status GenerateVariants(const SubcommandArgsRange& args, std::string source_contents; if (auto status = verible::file::GetContents(source_file, &source_contents); !status.ok()) { - std::cerr << "ERROR: passed file can't be open\n."; - return status; // check if the the file is not readable or doesn't exist. + message_stream << source_file << status; + return status; } // Lexing the input SV source code. @@ -171,12 +155,15 @@ static absl::Status GenerateVariants(const SubcommandArgsRange& args, verilog::FlowTree control_flow_tree(lexed_sequence); int counter = 0; auto status = control_flow_tree.GenerateVariants( - [limit_variants, &counter](const verilog::FlowTree::Variant& variant) { + [limit_variants, &outs, &message_stream, + &counter](const verilog::FlowTree::Variant& variant) { if (counter == limit_variants) return false; counter++; - std::cout << "Variant number " << counter << ":\n"; - for (auto token : variant.sequence) std::cout << token << '\n'; - puts(""); + message_stream << "Variant number " << counter << ":\n"; + for (auto token : variant.sequence) outs << token << '\n'; + // TODO(karimtera): Consider creating an output file per vairant, + // Such that the files naming reflects which defines are + // defined/undefined. return true; }); if (!status.ok()) { @@ -202,26 +189,32 @@ static const std::pair kCommands[] = { Output: (stdout) Contents of original file with // and /**/ comments removed. )"}}, - {"multiple-cu", + {"multiple-compilation-unit", {&MultipleCU, - R"(multiple-cu file [more_files] + R"(multiple-compilation-unit file [more_files] Inputs: 'file' is a Verilog or SystemVerilog source file. There can be multiple SystemVerilog source files. - Each one of them will be compiled in a separate compilation unit. + Each one of them will be prepropcessed separatly which means that declarations + scoopes will end by the end of each file, and won't be seen from other files. Output: (stdout) The preprocessed files content (same contents with directives interpreted). )"}}, {"generate-variants", {&GenerateVariants, - R"(genera-variants file [-limit_variants number] + R"(generate-variants file [-limit_variants number] Inputs: 'file' is a Verilog or SystemVerilog source file. '-limit_variants' flag limits variants to 'number' (20 by default). Output: (stdout) Generates every possible variant considering the conditional directives. )"}}, + // TODO(karimtera): We can add another argument to `generate-variants`, + // Which allows us to set some defines, as if we are only interested + // in the variants in which these defines are set. + // TODO(karimtera): Another candidate subcommand is `list-defines`, + // Which would be the output of `GetUsedMacros()`. }; int main(int argc, char* argv[]) { diff --git a/verilog/tools/preprocessor/verilog_preprocessor_test.sh b/verilog/tools/preprocessor/verilog_preprocessor_test.sh index 7c1704be6..6b06ab6ec 100755 --- a/verilog/tools/preprocessor/verilog_preprocessor_test.sh +++ b/verilog/tools/preprocessor/verilog_preprocessor_test.sh @@ -233,7 +233,7 @@ EOF status="$?" [[ $status == 0 ]] || { - "Expected exit code 1, but got $status" + "Expected exit code 0, but got $status" exit 0 } @@ -252,5 +252,243 @@ status="$?" "Expected exit code 1, but got $status" exit 1 } +################################################################################ +echo "=== Test multiple-compilation-unit: on a source file with conditionals" + +cat > "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_OUTPUT_FILE" + +status="$?" +[[ $status == 0 ]] || { + "Expected exit code 0, but got $status" + exit 0 +} + +diff --strip-trailing-cr -u "$MY_EXPECT_FILE" "$MY_OUTPUT_FILE" || { + exit 1 +} + +################################################################################ +echo "=== Test generate-variants: on a source file with nested conditionals" + +cat > "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_OUTPUT_FILE" 2>&1 + +status="$?" +[[ $status == 0 ]] || { + "Expected exit code 0, but got $status" + exit 0 +} + +diff --strip-trailing-cr -u "$MY_EXPECT_FILE" "$MY_OUTPUT_FILE" || { + exit 1 +} + +################################################################################ +echo "=== Test generate-variants: with limited variants" + +cat > "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_OUTPUT_FILE" 2>&1 + +status="$?" +[[ $status == 0 ]] || { + "Expected exit code 0, but got $status" + exit 0 +} + +diff --strip-trailing-cr -u "$MY_EXPECT_FILE" "$MY_OUTPUT_FILE" || { + exit 1 +} + +################################################################################ +echo "=== Test generate-variants: with invalid conditionals" + +cat > "$MY_INPUT_FILE" < "$MY_OUTPUT_FILE" 2>&1 + +status="$?" +[[ $status == 1 ]] || { + "Expected exit code 1, but got $status" + exit 0 +} + +################################################################################ +echo "=== Test generate-variants: with multiple files" + +cat > "$MY_INPUT_FILE" < "$MY_OUTPUT_FILE" 2>&1 + +status="$?" +[[ $status == 1 ]] || { + "Expected exit code 1, but got $status" + exit 0 +} + +################################################################################ +echo "=== Test multiple-compilation-unit: with missing files" + +"$preprocessor" multiple-compilation-unit > "$MY_OUTPUT_FILE" 2>&1 + +status="$?" +[[ $status == 1 ]] || { + "Expected exit code 1, but got $status" + exit 0 +} + +################################################################################ +echo "=== Test generate-variants: with multiple files" + +cat > "$MY_INPUT_FILE" < "$MY_OUTPUT_FILE" 2>&1 + +status="$?" +[[ $status == 1 ]] || { + "Expected exit code 1, but got $status" + exit 0 +} + +################################################################################ +echo "=== Test multiple-compilation-unit: with multiple files" + +cat > "$MY_INPUT_FILE" < "$MY_EXPECT_FILE" < "$MY_OUTPUT_FILE" + +status="$?" +[[ $status == 0 ]] || { + "Expected exit code 0, but got $status" + exit 0 +} + +diff --strip-trailing-cr -u "$MY_EXPECT_FILE" "$MY_OUTPUT_FILE" || { + exit 1 +} + ################################################################################ echo "PASS" From aa29595ea1be42440d7b39bfe889f4f3095e0f1c Mon Sep 17 00:00:00 2001 From: karimtera Date: Fri, 2 Sep 2022 01:34:09 +0200 Subject: [PATCH 15/17] fixing a minor rebasing issue. --- verilog/tools/preprocessor/verilog_preprocessor_test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/verilog/tools/preprocessor/verilog_preprocessor_test.sh b/verilog/tools/preprocessor/verilog_preprocessor_test.sh index 6b06ab6ec..d60e04fe7 100755 --- a/verilog/tools/preprocessor/verilog_preprocessor_test.sh +++ b/verilog/tools/preprocessor/verilog_preprocessor_test.sh @@ -158,6 +158,7 @@ endmodule EOF cat > "$MY_EXPECT_FILE" < Date: Fri, 2 Sep 2022 02:51:24 +0200 Subject: [PATCH 16/17] Adding tests to the preprocessor tool. --- verilog/analysis/flow_tree_test.cc | 87 +++++++++++++++++++ .../preprocessor/verilog_preprocessor.cc | 4 +- .../preprocessor/verilog_preprocessor_test.sh | 2 +- 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/verilog/analysis/flow_tree_test.cc b/verilog/analysis/flow_tree_test.cc index 0eb805ec5..8eb13f50a 100644 --- a/verilog/analysis/flow_tree_test.cc +++ b/verilog/analysis/flow_tree_test.cc @@ -199,5 +199,92 @@ TEST(FlowTree, NestedConditionals) { } } +TEST(FlowTree, MultipleElseIfs) { + const absl::string_view test_case = + R"( + `ifdef A + A_TRUE + `elsif B + B_TRUE + `elsif EMPTY + `elsif C + C_TRUE + `endif)"; + + FlowTree tree_test(LexToSequence(test_case)); + std::vector variants; + auto status = + tree_test.GenerateVariants([&variants](const FlowTree::Variant& variant) { + variants.push_back(variant); + return true; + }); + EXPECT_TRUE(status.ok()); + EXPECT_EQ(variants.size(), 5); + + const auto& used_macros = tree_test.GetUsedMacros(); + EXPECT_EQ(used_macros.size(), 4); + EXPECT_THAT(used_macros[0]->text(), "A"); + EXPECT_THAT(used_macros[1]->text(), "B"); + EXPECT_THAT(used_macros[2]->text(), "EMPTY"); + EXPECT_THAT(used_macros[3]->text(), "C"); + + // A is defined. + EXPECT_TRUE(variants[0].macros_mask.test(0)); + EXPECT_THAT(variants[0].sequence[0].text(), "A_TRUE"); + + // B is defined. + EXPECT_TRUE(variants[1].macros_mask.test(1)); + EXPECT_THAT(variants[1].sequence[0].text(), "B_TRUE"); + + // EMPTY is defined. + EXPECT_TRUE(variants[2].macros_mask.test(2)); + EXPECT_TRUE(variants[2].sequence.empty()); + + // C is defined. + EXPECT_TRUE(variants[3].macros_mask.test(3)); + EXPECT_THAT(variants[3].sequence[0].text(), "C_TRUE"); +} + +TEST(FlowTree, SwappedNegatedIfs) { + const absl::string_view test_case = + R"( + `ifndef A + A_FALSE + `elsif B + B_TRUE + `endif + + `ifndef B + B_FALSE + `elsif A + A_TRUE + `endif)"; + + FlowTree tree_test(LexToSequence(test_case)); + std::vector variants; + auto status = + tree_test.GenerateVariants([&variants](const FlowTree::Variant& variant) { + variants.push_back(variant); + return true; + }); + EXPECT_TRUE(status.ok()); + EXPECT_EQ(variants.size(), 4); + + const auto& used_macros = tree_test.GetUsedMacros(); + EXPECT_EQ(used_macros.size(), 2); + EXPECT_THAT(used_macros[0]->text(), "A"); + EXPECT_THAT(used_macros[1]->text(), "B"); + + EXPECT_THAT(variants[0].sequence[0].text(), "A_FALSE"); + EXPECT_THAT(variants[0].sequence[1].text(), "B_FALSE"); + + EXPECT_THAT(variants[1].sequence[0].text(), "A_FALSE"); + + EXPECT_THAT(variants[2].sequence[0].text(), "B_TRUE"); + EXPECT_THAT(variants[2].sequence[1].text(), "A_TRUE"); + + EXPECT_THAT(variants[3].sequence[0].text(), "B_FALSE"); +} + } // namespace } // namespace verilog diff --git a/verilog/tools/preprocessor/verilog_preprocessor.cc b/verilog/tools/preprocessor/verilog_preprocessor.cc index d47b83e01..b37e4084b 100644 --- a/verilog/tools/preprocessor/verilog_preprocessor.cc +++ b/verilog/tools/preprocessor/verilog_preprocessor.cc @@ -72,7 +72,7 @@ static absl::Status StripComments(const SubcommandArgsRange& args, return absl::OkStatus(); } -static absl::Status PreprocessSingleFile(const char* source_file, std::istream&, +static absl::Status PreprocessSingleFile(absl::string_view source_file, std::ostream& outs, std::ostream& message_stream) { std::string source_contents; @@ -115,7 +115,7 @@ static absl::Status MultipleCU(const SubcommandArgsRange& args, std::istream&, } for (const char* source_file : args) { message_stream << source_file << ":\n"; - auto status = PreprocessSingleFile(source_file, std::cin, outs, std::cerr); + auto status = PreprocessSingleFile(source_file, outs, message_stream); if (!status.ok()) return status; outs << '\n'; } diff --git a/verilog/tools/preprocessor/verilog_preprocessor_test.sh b/verilog/tools/preprocessor/verilog_preprocessor_test.sh index d60e04fe7..8373d087d 100755 --- a/verilog/tools/preprocessor/verilog_preprocessor_test.sh +++ b/verilog/tools/preprocessor/verilog_preprocessor_test.sh @@ -89,7 +89,7 @@ status="$?" exit 1 } -################################################################################################################################################################ +################################################################################ echo "=== Test strip-comments: white out comments" cat > "$MY_INPUT_FILE" < Date: Fri, 2 Sep 2022 04:01:29 +0200 Subject: [PATCH 17/17] Fixing a bug in adding the edges in FlowTree class. --- verilog/analysis/flow_tree.cc | 3 +- verilog/analysis/flow_tree_test.cc | 71 ++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/verilog/analysis/flow_tree.cc b/verilog/analysis/flow_tree.cc index 3007b2232..64b397040 100644 --- a/verilog/analysis/flow_tree.cc +++ b/verilog/analysis/flow_tree.cc @@ -88,7 +88,8 @@ absl::Status FlowTree::AddBlockEdges(const ConditionalBlock &block) { if (contains_elsif) { for (auto iter : block.elsif_locations) edges_[iter - 1].push_back(block.endif_location); - } else if (contains_else) { + } + if (contains_else) { edges_[block.else_location - 1].push_back(block.endif_location); } diff --git a/verilog/analysis/flow_tree_test.cc b/verilog/analysis/flow_tree_test.cc index 8eb13f50a..a34f90807 100644 --- a/verilog/analysis/flow_tree_test.cc +++ b/verilog/analysis/flow_tree_test.cc @@ -132,6 +132,36 @@ TEST(FlowTree, UnmatchedElses) { } } +TEST(FlowTree, UnvalidConditionals) { + const absl::string_view test_cases[] = { + R"( + `ifdef A + A_TRUE + `elsif + A_FALSE + )", + R"( + `ifdef + A_TRUE + `else + A_FALSE + )", + R"( + `ifndef + A_TRUE + `else + A_FALSE + `endif + )"}; + + for (auto test : test_cases) { + FlowTree tree_test(LexToSequence(test)); + auto status = tree_test.GenerateVariants( + [](const FlowTree::Variant& variant) { return true; }); + EXPECT_FALSE(status.ok()); + } +} + TEST(FlowTree, UncompletedConditionals) { const absl::string_view test_cases[] = { R"( @@ -286,5 +316,46 @@ TEST(FlowTree, SwappedNegatedIfs) { EXPECT_THAT(variants[3].sequence[0].text(), "B_FALSE"); } +TEST(FlowTree, CompleteConditional) { + const absl::string_view test_case = + R"( + `ifdef A + A_TRUE + `elsif B + B_TRUE + `elsif C + C_TRUE + `else + ALL_FALSE + `endif)"; + + FlowTree tree_test(LexToSequence(test_case)); + std::vector variants; + auto status = + tree_test.GenerateVariants([&variants](const FlowTree::Variant& variant) { + variants.push_back(variant); + return true; + }); + EXPECT_TRUE(status.ok()); + EXPECT_EQ(variants.size(), 4); + + const auto& used_macros = tree_test.GetUsedMacros(); + EXPECT_EQ(used_macros.size(), 3); + EXPECT_THAT(used_macros[0]->text(), "A"); + EXPECT_THAT(used_macros[1]->text(), "B"); + EXPECT_THAT(used_macros[2]->text(), "C"); + + EXPECT_TRUE(variants[0].macros_mask.test(0)); + EXPECT_THAT(variants[0].sequence[0].text(), "A_TRUE"); + + EXPECT_TRUE(variants[1].macros_mask.test(1)); + EXPECT_THAT(variants[1].sequence[0].text(), "B_TRUE"); + + EXPECT_TRUE(variants[2].macros_mask.test(2)); + EXPECT_THAT(variants[2].sequence[0].text(), "C_TRUE"); + + EXPECT_THAT(variants[3].sequence[0].text(), "ALL_FALSE"); +} + } // namespace } // namespace verilog