[OFFLOAD] Add support for more fine grained debug messages control #165416
Conversation
|
@pbalcer for awareness |
|
@llvm/pr-subscribers-offload Author: Alex Duran (adurang) ChangesThis PR introduces new debug macros that allow a more fined control of which debug message to output and introduce C++ stream syntax for debug messages. Changing existing messages (except a few that I changed for testing) will come in subsequent PRs. I also think that we should make debug enabling OpenMP agnostic but, for now, I prioritized maintaing the current libomptarget behavior for now, and we might need more changes further down the line as we we decouple libomptarget. Full diff: https://github.com/llvm/llvm-project/pull/165416.diff 4 Files Affected:
diff --git a/offload/include/Shared/Debug.h b/offload/include/Shared/Debug.h
index 7c3db8dbf119f..85e1c0e19445d 100644
--- a/offload/include/Shared/Debug.h
+++ b/offload/include/Shared/Debug.h
@@ -42,6 +42,8 @@
#include <mutex>
#include <string>
+#include "llvm/Support/circular_raw_ostream.h"
+
/// 32-Bit field data attributes controlling information presented to the user.
enum OpenMPInfoType : uint32_t {
// Print data arguments and attributes upon entering an OpenMP device kernel.
@@ -198,4 +200,189 @@ inline uint32_t getDebugLevel() {
} \
} while (false)
+// New macros that will allow for more granular control over debugging output
+// Each message can be classified by Component, Type and Level
+// Component: The broad component of the offload runtime emitting the message.
+// Type: A cross-component classification of messages
+// Level: The verbosity level of the message
+//
+// The component is pulled from the TARGET_NAME macro, Type and Level can be
+// defined for each debug message but by default they are "default" and "1"
+// respectively.
+//
+// For liboffload and plugins, use OFFLOAD_DEBUG(...)
+// For libomptarget, use OPENMP_DEBUG(...)
+// Constructing messages should be done using C++ stream style syntax.
+//
+// Usage examples:
+// OFFLOAD_DEBUG("type1", 2, "This is a level 2 message of type1");
+// OFFLOAD_DEBUG("Init", "This is a default level of the init type");
+// OPENMP_DEBUG("This is a level 1 message of the default type");
+// OFFLOAD_DEBUG("Init", 3, NumDevices << " were initialized\n");
+// OFFLOAD_DEBUG("Kernel", "Starting kernel " << KernelName << " on device " <<
+// DeviceId);
+//
+// Message output can be controlled by setting LIBOMPTARGET_DEBUG or
+// LIBOFFLOAD_DEBUG environment variables. Their syntax is as follows:
+// [integer]|all|<type1>[:<level1>][,<type2>[:<level2>],...]
+//
+// 0 : Disable all debug messages
+// all : Enable all debug messages
+// integer : Set the default level for all messages
+// <type> : Enable only messages of the specified type and level (more than one
+// can be specified). Components are also supported as
+// types.
+// <level> : Set the verbosity level for the specified type (default is 1)
+//
+// For very specific cases where more control is needed, use OFFLOAD_DEBUG_RAW
+// or OFFLOAD_DEBUG_BASE. See below for details.
+
+namespace llvm::offload::debug {
+
+#ifdef OMPTARGET_DEBUG
+
+struct DebugFilter {
+ StringRef Type;
+ uint32_t Level;
+};
+
+struct DebugSettings {
+ bool Enabled = false;
+ uint32_t DefaultLevel = 1;
+ llvm::SmallVector<DebugFilter> Filters;
+};
+
+/// dbgs - Return a circular-buffered debug stream.
+inline llvm::raw_ostream &dbgs() {
+ // Do one-time initialization in a thread-safe way.
+ static struct dbgstream {
+ llvm::circular_raw_ostream strm;
+
+ dbgstream() : strm(llvm::errs(), "*** Debug Log Output ***\n", 0) {}
+ } thestrm;
+
+ return thestrm.strm;
+}
+
+inline DebugFilter parseDebugFilter(StringRef Filter) {
+ size_t Pos = Filter.find(':');
+ if (Pos == StringRef::npos)
+ return {Filter, 1};
+
+ StringRef Type = Filter.slice(0, Pos);
+ uint32_t Level = 1;
+ if (Filter.slice(Pos + 1, Filter.size()).getAsInteger(10, Level))
+ Level = 1;
+
+ return {Type, Level};
+}
+
+inline DebugSettings &getDebugSettings() {
+ static DebugSettings Settings;
+ static std::once_flag Flag{};
+ std::call_once(Flag, []() {
+ printf("Configuring debug settings\n");
+ // Eventually, we probably should allow the upper layers to set
+ // debug settings directly according to their own env var or
+ // other methods.
+ // For now, mantain compatibility with existing libomptarget env var
+ // and add a liboffload independent one.
+ char *Env = getenv("LIBOMPTARGET_DEBUG");
+ if (!Env) {
+ Env = getenv("LIBOFFLOAD_DEBUG");
+ if (!Env)
+ return;
+ }
+
+ StringRef EnvRef(Env);
+ if (EnvRef == "0")
+ return;
+
+ Settings.Enabled = true;
+ if (EnvRef.equals_insensitive("all"))
+ return;
+
+ if (!EnvRef.getAsInteger(10, Settings.DefaultLevel))
+ return;
+
+ Settings.DefaultLevel = 1;
+
+ SmallVector<StringRef> DbgTypes;
+ EnvRef.split(DbgTypes, ',', -1, false);
+
+ for (auto &DT : DbgTypes)
+ Settings.Filters.push_back(parseDebugFilter(DT));
+ });
+
+ return Settings;
+}
+
+inline bool isDebugEnabled() { return getDebugSettings().Enabled; }
+
+inline bool shouldPrintDebug(const char *Component, const char *Type,
+ uint32_t Level) {
+ const auto &Settings = getDebugSettings();
+ if (!Settings.Enabled)
+ return false;
+
+ if (Settings.Filters.empty())
+ return Level <= Settings.DefaultLevel;
+
+ for (const auto &DT : Settings.Filters) {
+ if (DT.Level < Level)
+ continue;
+ if (DT.Type.equals_insensitive(Type))
+ return true;
+ if (DT.Type.equals_insensitive(Component))
+ return true;
+ }
+
+ return false;
+}
+
+#define OFFLOAD_DEBUG_BASE(Component, Type, Level, ...) \
+ do { \
+ if (llvm::offload::debug::isDebugEnabled() && \
+ llvm::offload::debug::shouldPrintDebug(Component, Type, Level)) \
+ __VA_ARGS__; \
+ } while (0)
+
+#define OFFLOAD_DEBUG_RAW(Type, Level, X) \
+ OFFLOAD_DEBUG_BASE(GETNAME(TARGET_NAME), Type, Level, X)
+
+#define OFFLOAD_DEBUG_1(X) \
+ OFFLOAD_DEBUG_BASE(GETNAME(TARGET_NAME), "default", 1, \
+ llvm::offload::debug::dbgs() \
+ << DEBUG_PREFIX << " --> " << X)
+
+#define OFFLOAD_DEBUG_2(Type, X) \
+ OFFLOAD_DEBUG_BASE(GETNAME(TARGET_NAME), Type, 1, \
+ llvm::offload::debug::dbgs() \
+ << DEBUG_PREFIX << " --> " << X)
+
+#define OFFLOAD_DEBUG_3(Type, Level, X) \
+ OFFLOAD_DEBUG_BASE(GETNAME(TARGET_NAME), Type, Level, \
+ llvm::offload::debug::dbgs() \
+ << DEBUG_PREFIX << " --> " << X)
+
+#define OFFLOAD_SELECT(Type, Level, X, NArgs, ...) OFFLOAD_DEBUG_##NArgs
+
+// To be used in liboffload and plugins
+#define OFFLOAD_DEBUG(...) OFFLOAD_SELECT(__VA_ARGS__, 3, 2, 1)(__VA_ARGS__)
+
+// To be used in libomptarget only
+#define OPENMP_DEBUG(...) OFFLOAD_DEBUG(__VA_ARGS__)
+
+#else
+
+// Don't print anything if debugging is disabled
+#define OFFLOAD_DEBUG_BASE(Component, Type, Level, ...)
+#define OFFLOAD_DEBUG_RAW(Type, Level, X)
+#define OFFLOAD_DEBUG(...)
+#define OPENMP_DEBUG(...)
+
+#endif
+
+} // namespace llvm::offload::debug
+
#endif // OMPTARGET_SHARED_DEBUG_H
diff --git a/offload/libomptarget/OffloadRTL.cpp b/offload/libomptarget/OffloadRTL.cpp
index 04bd21ec91a49..9f9ec5c8bfabe 100644
--- a/offload/libomptarget/OffloadRTL.cpp
+++ b/offload/libomptarget/OffloadRTL.cpp
@@ -35,7 +35,7 @@ void initRuntime() {
RefCount++;
if (RefCount == 1) {
- DP("Init offload library!\n");
+ OPENMP_DEBUG("Init offload library!\n");
#ifdef OMPT_SUPPORT
// Initialize OMPT first
llvm::omp::target::ompt::connectLibrary();
diff --git a/offload/libomptarget/PluginManager.cpp b/offload/libomptarget/PluginManager.cpp
index c8d6b42114d0f..915fd8df80eb7 100644
--- a/offload/libomptarget/PluginManager.cpp
+++ b/offload/libomptarget/PluginManager.cpp
@@ -36,7 +36,7 @@ void PluginManager::init() {
return;
}
- DP("Loading RTLs...\n");
+ OPENMP_DEBUG("Init", "Loading RTLs\n");
// Attempt to create an instance of each supported plugin.
#define PLUGIN_TARGET(Name) \
diff --git a/offload/plugins-nextgen/host/src/rtl.cpp b/offload/plugins-nextgen/host/src/rtl.cpp
index eb4ecac9907a1..3baadfebc541c 100644
--- a/offload/plugins-nextgen/host/src/rtl.cpp
+++ b/offload/plugins-nextgen/host/src/rtl.cpp
@@ -451,6 +451,8 @@ struct GenELF64PluginTy final : public GenericPluginTy {
if (auto Err = Plugin::check(ffi_init(), "failed to initialize libffi"))
return std::move(Err);
#endif
+ OFFLOAD_DEBUG("Init", 2,
+ "GenELF64 plugin detected" << NUM_DEVICES << " devices\n");
return NUM_DEVICES;
}
|
saiislam
left a comment
saiislam
left a comment
There was a problem hiding this comment.
Seems like a good idea, but have some questions. Please see inline.
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp,h -- offload/include/Shared/Debug.h offload/libomptarget/OffloadRTL.cpp offload/libomptarget/PluginManager.cpp offload/plugins-nextgen/host/src/rtl.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/offload/include/Shared/Debug.h b/offload/include/Shared/Debug.h
index 5d918f9af..5ca79040c 100644
--- a/offload/include/Shared/Debug.h
+++ b/offload/include/Shared/Debug.h
@@ -241,7 +241,7 @@ inline uint32_t getDebugLevel() {
// LIBOFFLOAD_DEBUG=init:3,mapping:2 (Print messages of type "init" of level 3
// or lower and messages of type "mapping" of
// level 2 or lower)
-// LIBOFFLOAD_DEBUG=omptarget:4, init (Print messages from component "omptarget"
+// LIBOFFLOAD_DEBUG=omptarget:4, init (Print messages from component "omptarget"
// of level 4 or lower and messages of type
// "init" of level 1 or lower)
//
|
| // other methods. | ||
| // For now, mantain compatibility with existing libomptarget env var | ||
| // and add a liboffload independent one. | ||
| char *Env = getenv("LIBOMPTARGET_DEBUG"); |
There was a problem hiding this comment.
I think we have environment variable helpers somewhere, can probably use those instead of raw getenv.
There was a problem hiding this comment.
This is probably the only place where they can't be used, because those helpers also have debug messages and it would create a circular dependence.
| if (auto Err = Plugin::check(ffi_init(), "failed to initialize libffi")) | ||
| return std::move(Err); | ||
| #endif | ||
| OFFLOAD_DEBUG("Init", 2, |
There was a problem hiding this comment.
I'm not a huge fan of this level concept, it feels totally arbitrary and just providing specific kinds gives people the tools required to recreate it.
There was a problem hiding this comment.
I agree that you can kind of recreate it with kinds, but not having levels is not as intuitive or easy to use as a user.
Say you have 3 levels for Init. Of course you can create "Init1", "Init2", "Init3". But now you need to list of all of them get the behavior as just "Init:3". This of course because more verbose with more Kinds and more Levels. Furthermore, there's no easy way to say all "levels" of a Kind unless we want to support more complicated comparisons like "Init*" (which I personally don't think we should) but supporting syntax like "Init:*" is trivial.
IMO, yes, levels are not as fundamentally useful as Kinds (there's no dispute to that) but the complexity/benefit the provide is worth it. Furthemore, they're optional so it's not like we need to start specifying them everywhere but just where it might make sense to provide different levels of detail for the same Kind of messages.
|
Have you considered to model this after https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/DebugLog.h? That is, LDBG() << "message";instead of Besides being shorter, advantages include is that it works better with code-completion than in a macro argument, and you do not get "unused variable" warnings in non-assert builds. LDBG() expands to Footnotes
|
That'd probably work well, we could make it like |
I don't remember seeing LDBG when I looked at that file. Sounds like a good suggestion, thanks @Meinersbur. I'll give it a try. |
5 participants
This PR introduces new debug macros that allow a more fined control of which debug message to output and introduce C++ stream syntax for debug messages.
Changing existing messages (except a few that I changed for testing) will come in subsequent PRs.
I also think that we should make debug enabling OpenMP agnostic but, for now, I prioritized maintaing the current libomptarget behavior for now, and we might need more changes further down the line as we we decouple libomptarget.