From afb530be9f9307d2edff76d7cf98197bffd42327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Thu, 18 Apr 2024 20:13:52 +0200 Subject: [PATCH 1/8] Contribute DiagnosticCountInPassHook --- ir/CMakeLists.txt | 2 ++ ir/pass_utils.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++++++ ir/pass_utils.h | 35 ++++++++++++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 ir/pass_utils.cpp create mode 100644 ir/pass_utils.h diff --git a/ir/CMakeLists.txt b/ir/CMakeLists.txt index b5410d6614..16569df200 100644 --- a/ir/CMakeLists.txt +++ b/ir/CMakeLists.txt @@ -27,6 +27,7 @@ set (IR_SRCS json_parser.cpp node.cpp pass_manager.cpp + pass_utils.cpp type.cpp v1.cpp visitor.cpp @@ -50,6 +51,7 @@ set (IR_HDRS node.h nodemap.h pass_manager.h + pass_utils.h vector.h visitor.h ) diff --git a/ir/pass_utils.cpp b/ir/pass_utils.cpp new file mode 100644 index 0000000000..279bd28b05 --- /dev/null +++ b/ir/pass_utils.cpp @@ -0,0 +1,64 @@ +#include "ir/pass_utils.h" + +#include +#include + +#include "lib/log.h" + +/// @file +/// @authors Vladimír Štill +/// @brief Utilities for passes and pass managers. + +namespace P4 { + +// The state needs to be shared between all hook copies as the hook is copied in the pass manager. +struct DiagnosticCountInPassHookState { + explicit DiagnosticCountInPassHookState(BaseCompileContext &ctxt) + : lastDiagCount(ctxt.errorReporter().getDiagnosticCount()), + lastErrorCount(ctxt.errorReporter().getErrorCount()), + lastWarningCount(ctxt.errorReporter().getWarningCount()), + lastInfoCount(ctxt.errorReporter().getInfoCount()) {} + + unsigned lastDiagCount, lastErrorCount, lastWarningCount, lastInfoCount; +}; + +DebugHook getDiagnosticCountInPassHook(BaseCompileContext &ctxt) { + return [state = std::make_shared(ctxt), &ctxt]( + const char *manager, unsigned seqNo, const char *pass, const IR::Node *) mutable { + unsigned diag = ctxt.errorReporter().getDiagnosticCount(); + + if (diag != state->lastDiagCount) { + unsigned errs = ctxt.errorReporter().getErrorCount(), + warns = ctxt.errorReporter().getWarningCount(), + infos = ctxt.errorReporter().getInfoCount(); + std::stringstream summary; + const char *sep = ""; + for (auto [newCnt, oldCnt, kind] : + {std::tuple(errs, state->lastErrorCount, "error"), + std::tuple(warns, state->lastWarningCount, "warning"), + std::tuple(infos, state->lastInfoCount, "info")}) { + if (newCnt > oldCnt) { + auto diff = newCnt - oldCnt; + summary << sep << diff << " " << kind << (diff > 1 ? "s" : ""); + sep = ", "; + } + } + + // if the log level is high enough, emit also pass count + if (Log::fileLogLevelIsAtLeast(DIAGNOSTIC_COUNT_IN_PASS_TAG, 4)) { + LOG_FEATURE(DIAGNOSTIC_COUNT_IN_PASS_TAG, 4, + "PASS " << manager << "[" << seqNo << "] ~> " << pass << " emitted " + << summary.str()); + } else { + LOG_FEATURE(DIAGNOSTIC_COUNT_IN_PASS_TAG, 1, + "PASS " << manager << " ~> " << pass << " emitted " << summary.str()); + } + state->lastDiagCount = diag; + state->lastErrorCount = errs; + state->lastWarningCount = warns; + state->lastInfoCount = infos; + } + }; +} + +} // namespace P4 diff --git a/ir/pass_utils.h b/ir/pass_utils.h new file mode 100644 index 0000000000..4929c08cf4 --- /dev/null +++ b/ir/pass_utils.h @@ -0,0 +1,35 @@ +#ifndef IR_PASS_UTILS_H_ +#define IR_PASS_UTILS_H_ + +#include "ir/pass_manager.h" +#include "lib/compile_context.h" + +/// @file +/// @authors Vladimír Štill +/// @brief Utilities for passes and pass managers. + +namespace P4 { + +inline const char *DIAGNOSTIC_COUNT_IN_PASS_TAG = "diagnosticCountInPass"; + +/// @brief This creates a debug hook that prints information about the number of diagnostics of +/// different levels (error, warning, info) printed by each of the pass after it ended. This is +/// intended to help in debuggig and testing. +/// +/// NOTE: You either need to create one pass and use its copies in all the pass managers in the +/// compilation, or create each of the following hooks only after the previous compilations teps had +/// already run. Otherwise, you can get spurious logs for a first pass of a pass manager running +/// after some diagnostics were emitted. +/// +/// It logs the messages if the log level for "diagnosticCountInPass" is at least 1. If the log +/// level is at least 4, the logs also include the sequence number of the pass that printed the +/// message in the pass manager. +/// +/// @param ctxt Optionally, you can provide a compilation context to take the diagnostic counts +/// from. If not provied BaseCompileContext::get() is used. +/// @return A debug hook suitable for using in pass managers. +DebugHook getDiagnosticCountInPassHook(BaseCompileContext &ctxt = BaseCompileContext::get()); + +} // namespace P4 + +#endif // IR_PASS_UTILS_H_ From 95a0c0ab58c81cb5bd8862ae71e16d00246c2596 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Tue, 23 Apr 2024 08:45:14 +0200 Subject: [PATCH 2/8] Update comment. Co-authored-by: Fabian Ruffy <5960321+fruffy@users.noreply.github.com> --- ir/pass_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ir/pass_utils.cpp b/ir/pass_utils.cpp index 279bd28b05..84032977e4 100644 --- a/ir/pass_utils.cpp +++ b/ir/pass_utils.cpp @@ -44,7 +44,7 @@ DebugHook getDiagnosticCountInPassHook(BaseCompileContext &ctxt) { } } - // if the log level is high enough, emit also pass count + // If the log level is high enough, emit also pass count. if (Log::fileLogLevelIsAtLeast(DIAGNOSTIC_COUNT_IN_PASS_TAG, 4)) { LOG_FEATURE(DIAGNOSTIC_COUNT_IN_PASS_TAG, 4, "PASS " << manager << "[" << seqNo << "] ~> " << pass << " emitted " From 948283658d7aa20863cf836561d150badeb0d4ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Tue, 23 Apr 2024 08:56:14 +0200 Subject: [PATCH 3/8] p4test: Add DiagnosticCountInPassHook has to be activated by -TdiagnosticCountInPass:1 to run --- backends/p4test/p4test.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backends/p4test/p4test.cpp b/backends/p4test/p4test.cpp index bb9e81f570..87115032cf 100644 --- a/backends/p4test/p4test.cpp +++ b/backends/p4test/p4test.cpp @@ -26,6 +26,7 @@ limitations under the License. #include "frontends/p4/toP4/toP4.h" #include "ir/ir.h" #include "ir/json_loader.h" +#include "ir/pass_utils.h" #include "lib/crash.h" #include "lib/error.h" #include "lib/exceptions.h" @@ -137,6 +138,9 @@ int main(int argc, char *const argv[]) { try { P4::FrontEnd fe; fe.addDebugHook(hook); + // use -TdiagnosticCountInPass:1 / -TdiagnosticCountInPass:4 to get output of + // this hook + fe.addDebugHook(P4::getDiagnosticCountInPassHook()); program = fe.run(options, program); } catch (const std::exception &bug) { std::cerr << bug.what() << std::endl; From f0e43844ddbe456bde05436134dc178ec9c150d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Tue, 23 Apr 2024 11:18:53 +0200 Subject: [PATCH 4/8] Extend interface for diagnosric count logs to allow more flexibility --- ir/pass_utils.cpp | 79 +++++++++++++++++++++++++++++++---------------- ir/pass_utils.h | 58 +++++++++++++++++++++++++++++++--- 2 files changed, 107 insertions(+), 30 deletions(-) diff --git a/ir/pass_utils.cpp b/ir/pass_utils.cpp index 84032977e4..fb3d28b0ff 100644 --- a/ir/pass_utils.cpp +++ b/ir/pass_utils.cpp @@ -3,7 +3,9 @@ #include #include +#include "absl/strings/str_cat.h" #include "lib/log.h" +#include "pass_utils.h" /// @file /// @authors Vladimír Štill @@ -12,31 +14,28 @@ namespace P4 { // The state needs to be shared between all hook copies as the hook is copied in the pass manager. -struct DiagnosticCountInPassHookState { - explicit DiagnosticCountInPassHookState(BaseCompileContext &ctxt) - : lastDiagCount(ctxt.errorReporter().getDiagnosticCount()), +struct DiagnosticCountInfoState { + explicit DiagnosticCountInfoState(BaseCompileContext &ctxt) + : ctxt(ctxt), + lastDiagCount(ctxt.errorReporter().getDiagnosticCount()), lastErrorCount(ctxt.errorReporter().getErrorCount()), lastWarningCount(ctxt.errorReporter().getWarningCount()), lastInfoCount(ctxt.errorReporter().getInfoCount()) {} - unsigned lastDiagCount, lastErrorCount, lastWarningCount, lastInfoCount; -}; + void info(std::string_view componentInfo) { + if (!Log::fileLogLevelIsAtLeast(DIAGNOSTIC_COUNT_IN_PASS_TAG, 1)) return; -DebugHook getDiagnosticCountInPassHook(BaseCompileContext &ctxt) { - return [state = std::make_shared(ctxt), &ctxt]( - const char *manager, unsigned seqNo, const char *pass, const IR::Node *) mutable { unsigned diag = ctxt.errorReporter().getDiagnosticCount(); - if (diag != state->lastDiagCount) { + if (diag != lastDiagCount) { unsigned errs = ctxt.errorReporter().getErrorCount(), warns = ctxt.errorReporter().getWarningCount(), infos = ctxt.errorReporter().getInfoCount(); std::stringstream summary; const char *sep = ""; - for (auto [newCnt, oldCnt, kind] : - {std::tuple(errs, state->lastErrorCount, "error"), - std::tuple(warns, state->lastWarningCount, "warning"), - std::tuple(infos, state->lastInfoCount, "info")}) { + for (auto [newCnt, oldCnt, kind] : {std::tuple(errs, lastErrorCount, "error"), + std::tuple(warns, lastWarningCount, "warning"), + std::tuple(infos, lastInfoCount, "info")}) { if (newCnt > oldCnt) { auto diff = newCnt - oldCnt; summary << sep << diff << " " << kind << (diff > 1 ? "s" : ""); @@ -44,21 +43,49 @@ DebugHook getDiagnosticCountInPassHook(BaseCompileContext &ctxt) { } } - // If the log level is high enough, emit also pass count. - if (Log::fileLogLevelIsAtLeast(DIAGNOSTIC_COUNT_IN_PASS_TAG, 4)) { - LOG_FEATURE(DIAGNOSTIC_COUNT_IN_PASS_TAG, 4, - "PASS " << manager << "[" << seqNo << "] ~> " << pass << " emitted " - << summary.str()); - } else { - LOG_FEATURE(DIAGNOSTIC_COUNT_IN_PASS_TAG, 1, - "PASS " << manager << " ~> " << pass << " emitted " << summary.str()); - } - state->lastDiagCount = diag; - state->lastErrorCount = errs; - state->lastWarningCount = warns; - state->lastInfoCount = infos; + LOG_FEATURE(DIAGNOSTIC_COUNT_IN_PASS_TAG, 1, + componentInfo << " emitted " << summary.str()); + lastDiagCount = diag; + lastErrorCount = errs; + lastWarningCount = warns; + lastInfoCount = infos; + } + } + + BaseCompileContext &ctxt; + unsigned lastDiagCount, lastErrorCount, lastWarningCount, lastInfoCount; +}; + +static DebugHook hook(std::shared_ptr state) { + return [state](const char *manager, unsigned seqNo, const char *pass, const IR::Node *) { + if (!Log::fileLogLevelIsAtLeast(DIAGNOSTIC_COUNT_IN_PASS_TAG, 1)) return; + + std::string compInfo = absl::StrCat("PASS ", manager); + // If the log level is high enough, emit also pass count. + if (Log::fileLogLevelIsAtLeast(DIAGNOSTIC_COUNT_IN_PASS_TAG, 4)) { + absl::StrAppend(&compInfo, "[", seqNo, "]"); } + absl::StrAppend(&compInfo, " ~> ", pass); + + state->info(compInfo); }; } +DebugHook getDiagnosticCountInPassHook(BaseCompileContext &ctxt) { + return hook(std::make_shared(ctxt)); +} + +DiagnosticCountInfoGuard::~DiagnosticCountInfoGuard() { state->info(componentInfo); } + +DiagnosticCountInfo::DiagnosticCountInfo(BaseCompileContext &ctxt) + : state(std::make_shared(ctxt)) {} + +DebugHook DiagnosticCountInfo::getPassManagerHook() { return hook(state); } + +void DiagnosticCountInfo::emitInfo(std::string_view componentInfo) { state->info(componentInfo); } + +DiagnosticCountInfoGuard DiagnosticCountInfo::getInfoGuard(std::string_view componentInfo) { + return {componentInfo, state}; +} + } // namespace P4 diff --git a/ir/pass_utils.h b/ir/pass_utils.h index 4929c08cf4..bd1155b2b3 100644 --- a/ir/pass_utils.h +++ b/ir/pass_utils.h @@ -16,10 +16,11 @@ inline const char *DIAGNOSTIC_COUNT_IN_PASS_TAG = "diagnosticCountInPass"; /// different levels (error, warning, info) printed by each of the pass after it ended. This is /// intended to help in debuggig and testing. /// -/// NOTE: You either need to create one pass and use its copies in all the pass managers in the -/// compilation, or create each of the following hooks only after the previous compilations teps had -/// already run. Otherwise, you can get spurious logs for a first pass of a pass manager running -/// after some diagnostics were emitted. +/// NOTE: You either need to use one \ref DiagnosticCountInfo object (and its \ref +/// getPassManagerHook) for the entire compilation, create one pass and use its copies in all the +/// pass managers in the compilation, or create each of the following hooks only after the previous +/// compilations steps had already run. Otherwise, you can get spurious logs for a first pass of a +/// pass manager running after some diagnostics were emitted. /// /// It logs the messages if the log level for "diagnosticCountInPass" is at least 1. If the log /// level is at least 4, the logs also include the sequence number of the pass that printed the @@ -30,6 +31,55 @@ inline const char *DIAGNOSTIC_COUNT_IN_PASS_TAG = "diagnosticCountInPass"; /// @return A debug hook suitable for using in pass managers. DebugHook getDiagnosticCountInPassHook(BaseCompileContext &ctxt = BaseCompileContext::get()); +struct DiagnosticCountInfoState; // forward declaration + +/// A guard useful for emitting diagnostic info about non-passes. +/// \see DiagnosticCountInfo::getInfoGuard. +struct DiagnosticCountInfoGuard { + ~DiagnosticCountInfoGuard(); + + /// avoid creating moved-from object and copies that could emit spurious logs. + DiagnosticCountInfoGuard(DiagnosticCountInfoGuard &&) = delete; + + friend struct DiagnosticCountInfo; + + private: + DiagnosticCountInfoGuard(std::string_view componentInfo, + std::shared_ptr state) + : componentInfo(componentInfo), state(state) {} + std::string_view componentInfo; + std::shared_ptr state; +}; + +/// An alternative interface to the diagnostic count info that allows using it outside of pass +/// manager (but can also generate a pass manager hook). +/// All copies of one object and all hooks and guards derived from it share the same diagnostic +/// message counts so they can provide consistent logs. +/// +/// \see getDiagnosticCountInPassHook +struct DiagnosticCountInfo { + /// @param ctxt Optionally, you can provide a compilation context to take the diagnostic counts + /// from. If not provied BaseCompileContext::get() is used. + DiagnosticCountInfo(BaseCompileContext &ctxt = BaseCompileContext::get()); + + /// Very similar to \ref getDiagnosticCountInPassHook, but the state is tied with this instance + /// so that calling the hook and the \ref emitInfo/\ref getInfoGuard during one compilation will + /// produce the correct counts. + DebugHook getPassManagerHook(); + + /// Emits the information like printed by the debug hook, except using componentInfo as the info + /// at the beginning of the line: the line will be in form " emitted ". + /// This is useful e.g. for printing info about diagnostics in parser. + void emitInfo(std::string_view componentInfo); + + /// Similar to \ref emitInfo, but prints the info at the moment the returned guard is + /// destructed. + DiagnosticCountInfoGuard getInfoGuard(std::string_view componentInfo); + + private: + std::shared_ptr state; +}; + } // namespace P4 #endif // IR_PASS_UTILS_H_ From 36b11f3eadf0a8fec7e9c7b222bc266652e10eb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Tue, 23 Apr 2024 11:19:26 +0200 Subject: [PATCH 5/8] p4test: Use the update logs --- backends/p4test/p4test.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/backends/p4test/p4test.cpp b/backends/p4test/p4test.cpp index 87115032cf..2656084f18 100644 --- a/backends/p4test/p4test.cpp +++ b/backends/p4test/p4test.cpp @@ -128,11 +128,14 @@ int main(int argc, char *const argv[]) { error(ErrorType::ERR_IO, "Can't open %s", options.file); } } else { + P4::DiagnosticCountInfo info; program = P4::parseP4File(options); + info.emitInfo("PARSER"); if (program != nullptr && ::errorCount() == 0) { P4::P4COptionPragmaParser optionsPragmaParser; program->apply(P4::ApplyOptionsPragmas(optionsPragmaParser)); + info.emitInfo("PASS P4COptionPragmaParser"); if (!options.parseOnly) { try { @@ -140,7 +143,7 @@ int main(int argc, char *const argv[]) { fe.addDebugHook(hook); // use -TdiagnosticCountInPass:1 / -TdiagnosticCountInPass:4 to get output of // this hook - fe.addDebugHook(P4::getDiagnosticCountInPassHook()); + fe.addDebugHook(info.getPassManagerHook()); program = fe.run(options, program); } catch (const std::exception &bug) { std::cerr << bug.what() << std::endl; From a65fa662f58ea94fafa5683c381382b3e52cd37f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Tue, 23 Apr 2024 14:53:13 +0200 Subject: [PATCH 6/8] Make ctor explicit --- ir/pass_utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ir/pass_utils.h b/ir/pass_utils.h index bd1155b2b3..91b6b74e52 100644 --- a/ir/pass_utils.h +++ b/ir/pass_utils.h @@ -60,7 +60,7 @@ struct DiagnosticCountInfoGuard { struct DiagnosticCountInfo { /// @param ctxt Optionally, you can provide a compilation context to take the diagnostic counts /// from. If not provied BaseCompileContext::get() is used. - DiagnosticCountInfo(BaseCompileContext &ctxt = BaseCompileContext::get()); + explicit DiagnosticCountInfo(BaseCompileContext &ctxt = BaseCompileContext::get()); /// Very similar to \ref getDiagnosticCountInPassHook, but the state is tied with this instance /// so that calling the hook and the \ref emitInfo/\ref getInfoGuard during one compilation will From 84a7d830b4d57ac21002701ba3cc5de06b2821f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Tue, 23 Apr 2024 16:59:38 +0200 Subject: [PATCH 7/8] Drop the authors doxygen tag that is not used in other files --- ir/pass_utils.cpp | 1 - ir/pass_utils.h | 1 - 2 files changed, 2 deletions(-) diff --git a/ir/pass_utils.cpp b/ir/pass_utils.cpp index fb3d28b0ff..54218678b4 100644 --- a/ir/pass_utils.cpp +++ b/ir/pass_utils.cpp @@ -8,7 +8,6 @@ #include "pass_utils.h" /// @file -/// @authors Vladimír Štill /// @brief Utilities for passes and pass managers. namespace P4 { diff --git a/ir/pass_utils.h b/ir/pass_utils.h index 91b6b74e52..ad74216dae 100644 --- a/ir/pass_utils.h +++ b/ir/pass_utils.h @@ -5,7 +5,6 @@ #include "lib/compile_context.h" /// @file -/// @authors Vladimír Štill /// @brief Utilities for passes and pass managers. namespace P4 { From 8c71374dfe72b8a61a84ec955e263938ad06fa26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C5=A0till?= Date: Thu, 25 Apr 2024 08:52:02 +0200 Subject: [PATCH 8/8] Add copyring headers --- ir/pass_utils.cpp | 16 ++++++++++++++++ ir/pass_utils.h | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/ir/pass_utils.cpp b/ir/pass_utils.cpp index 54218678b4..be4ab96797 100644 --- a/ir/pass_utils.cpp +++ b/ir/pass_utils.cpp @@ -1,3 +1,19 @@ +/* +Copyright 2024 Intel Corp. + +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 "ir/pass_utils.h" #include diff --git a/ir/pass_utils.h b/ir/pass_utils.h index ad74216dae..de593eb3b9 100644 --- a/ir/pass_utils.h +++ b/ir/pass_utils.h @@ -1,3 +1,19 @@ +/* +Copyright 2024 Intel Corp. + +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 IR_PASS_UTILS_H_ #define IR_PASS_UTILS_H_