[Offload] Enable more refined debug printing #163431
Conversation
There are some users who use debug build and/or enable debug printing but are only interested in a subset of the emitted messages. This change addresses such needs by extending the existing environment variable `LIBOMPTARGET_DEBUG` which accepts both numeric and string values that represent subsets of the printed message. The behavior when it is a numeric value did not change.
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-offload Author: Hansang Bae (hansangbae) ChangesThere are some users who use debug build and/or enable debug printing but are only interested in a subset of the emitted messages. This change addresses such needs by extending the existing environment variable Patch is 123.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163431.diff 24 Files Affected:
diff --git a/offload/include/OffloadPolicy.h b/offload/include/OffloadPolicy.h
index 800fefb224326..d794376f2b59e 100644
--- a/offload/include/OffloadPolicy.h
+++ b/offload/include/OffloadPolicy.h
@@ -37,12 +37,12 @@ class OffloadPolicy {
return;
default:
if (PM.getNumDevices()) {
- DP("Default TARGET OFFLOAD policy is now mandatory "
- "(devices were found)\n");
+ DPIF(RTL, "Default TARGET OFFLOAD policy is now mandatory "
+ "(devices were found)\n");
Kind = MANDATORY;
} else {
- DP("Default TARGET OFFLOAD policy is now disabled "
- "(no devices were found)\n");
+ DPIF(RTL, "Default TARGET OFFLOAD policy is now disabled "
+ "(no devices were found)\n");
Kind = DISABLED;
}
return;
diff --git a/offload/include/OpenMP/OMPT/Connector.h b/offload/include/OpenMP/OMPT/Connector.h
index c7b37740d5642..d37ea07e62166 100644
--- a/offload/include/OpenMP/OMPT/Connector.h
+++ b/offload/include/OpenMP/OMPT/Connector.h
@@ -76,7 +76,7 @@ class OmptLibraryConnectorTy {
std::string LibName = LibIdent;
LibName += ".so";
- DP("OMPT: Trying to load library %s\n", LibName.c_str());
+ DPIF(TOOL, "OMPT: Trying to load library %s\n", LibName.c_str());
auto DynLibHandle = std::make_unique<llvm::sys::DynamicLibrary>(
llvm::sys::DynamicLibrary::getPermanentLibrary(LibName.c_str(),
&ErrMsg));
@@ -85,12 +85,12 @@ class OmptLibraryConnectorTy {
LibConnHandle = nullptr;
} else {
auto LibConnRtn = "ompt_" + LibIdent + "_connect";
- DP("OMPT: Trying to get address of connection routine %s\n",
- LibConnRtn.c_str());
+ DPIF(TOOL, "OMPT: Trying to get address of connection routine %s\n",
+ LibConnRtn.c_str());
LibConnHandle = reinterpret_cast<OmptConnectRtnTy>(
DynLibHandle->getAddressOfSymbol(LibConnRtn.c_str()));
}
- DP("OMPT: Library connection handle = %p\n", LibConnHandle);
+ DPIF(TOOL, "OMPT: Library connection handle = %p\n", LibConnHandle);
IsInitialized = true;
}
diff --git a/offload/include/Shared/Debug.h b/offload/include/Shared/Debug.h
index 7c3db8dbf119f..b16bd5ea0f4a2 100644
--- a/offload/include/Shared/Debug.h
+++ b/offload/include/Shared/Debug.h
@@ -40,6 +40,7 @@
#include <atomic>
#include <mutex>
+#include <sstream>
#include <string>
/// 32-Bit field data attributes controlling information presented to the user.
@@ -62,6 +63,38 @@ enum OpenMPInfoType : uint32_t {
OMP_INFOTYPE_ALL = 0xffffffff,
};
+/// 32-bit field attributes controlling debug trace/dump
+enum DebugInfoType : uint32_t {
+ /// Generic plugin/runtime interface/management
+ DEBUG_INFOTYPE_RTL = 0x0001,
+ /// Generic device activity
+ DEBUG_INFOTYPE_DEVICE = 0x0002,
+ /// Module preparation
+ DEBUG_INFOTYPE_MODULE = 0x0004,
+ /// Kernel preparation and invocation
+ DEBUG_INFOTYPE_KERNEL = 0x0008,
+ /// Memory allocation/deallocation or related activities
+ DEBUG_INFOTYPE_MEMORY = 0x0010,
+ /// Data-mapping activities
+ DEBUG_INFOTYPE_MAP = 0x0020,
+ /// Data-copying or similar activities
+ DEBUG_INFOTYPE_COPY = 0x0040,
+ /// OpenMP interop
+ DEBUG_INFOTYPE_INTEROP = 0x0080,
+ /// Tool interface
+ DEBUG_INFOTYPE_TOOL = 0x0100,
+ /// Backend API tracing
+ DEBUG_INFOTYPE_API = 0x0200,
+ /// All
+ DEBUG_INFOTYPE_ALL = 0xffffffff,
+};
+
+/// Debug option struct to support both numeric and string value
+struct DebugOptionTy {
+ uint32_t Level;
+ uint32_t Type;
+};
+
inline std::atomic<uint32_t> &getInfoLevelInternal() {
static std::atomic<uint32_t> InfoLevel;
static std::once_flag Flag{};
@@ -75,17 +108,45 @@ inline std::atomic<uint32_t> &getInfoLevelInternal() {
inline uint32_t getInfoLevel() { return getInfoLevelInternal().load(); }
-inline uint32_t getDebugLevel() {
- static uint32_t DebugLevel = 0;
- static std::once_flag Flag{};
- std::call_once(Flag, []() {
- if (char *EnvStr = getenv("LIBOMPTARGET_DEBUG"))
- DebugLevel = std::stoi(EnvStr);
- });
-
- return DebugLevel;
+inline DebugOptionTy &getDebugOption() {
+ static DebugOptionTy DebugOption = []() {
+ DebugOptionTy OptVal{0, 0};
+ char *EnvStr = getenv("LIBOMPTARGET_DEBUG");
+ if (!EnvStr || *EnvStr == '0')
+ return OptVal; // undefined or explicitly defined as zero
+ OptVal.Level = std::atoi(EnvStr);
+ if (OptVal.Level)
+ return OptVal; // defined as numeric value
+ struct DebugStrToBitTy {
+ const char *Str;
+ uint32_t Bit;
+ } DebugStrToBit[] = {
+ {"rtl", DEBUG_INFOTYPE_RTL}, {"device", DEBUG_INFOTYPE_DEVICE},
+ {"module", DEBUG_INFOTYPE_MODULE}, {"kernel", DEBUG_INFOTYPE_KERNEL},
+ {"memory", DEBUG_INFOTYPE_MEMORY}, {"map", DEBUG_INFOTYPE_MAP},
+ {"copy", DEBUG_INFOTYPE_COPY}, {"interop", DEBUG_INFOTYPE_INTEROP},
+ {"tool", DEBUG_INFOTYPE_TOOL}, {"api", DEBUG_INFOTYPE_API},
+ {"all", DEBUG_INFOTYPE_ALL}, {nullptr, 0},
+ };
+ // Check string value of the option. Comma-separated list of the known
+ // keywords are accepted.
+ std::istringstream Tokens(EnvStr);
+ for (std::string Token; std::getline(Tokens, Token, ',');) {
+ for (int I = 0; DebugStrToBit[I].Str; I++) {
+ if (Token == DebugStrToBit[I].Str) {
+ OptVal.Type |= DebugStrToBit[I].Bit;
+ break;
+ }
+ }
+ }
+ return OptVal;
+ }();
+ return DebugOption;
}
+inline uint32_t getDebugLevel() { return getDebugOption().Level; }
+inline uint32_t getDebugType() { return getDebugOption().Type; }
+
#undef USED
#undef GCC_VERSION
@@ -154,18 +215,25 @@ inline uint32_t getDebugLevel() {
fprintf(stderr, __VA_ARGS__); \
}
-/// Emit a message for debugging
-#define DP(...) \
+/// Check if debug option is turned on for `Type`
+#define DPSET(Type) \
+ ((getDebugType() & DEBUG_INFOTYPE_##Type) || getDebugLevel() > 0)
+
+/// Emit a message for debugging if related to `Type`
+#define DPIF(Type, ...) \
do { \
- if (getDebugLevel() > 0) { \
+ if (DPSET(Type)) { \
DEBUGP(DEBUG_PREFIX, __VA_ARGS__); \
} \
} while (false)
+/// Emit a message for debugging
+#define DP(...) DPIF(ALL, __VA_ARGS__);
+
/// Emit a message for debugging or failure if debugging is disabled
#define REPORT(...) \
do { \
- if (getDebugLevel() > 0) { \
+ if (DPSET(ALL)) { \
DP(__VA_ARGS__); \
} else { \
FAILURE_MESSAGE(__VA_ARGS__); \
@@ -174,15 +242,45 @@ inline uint32_t getDebugLevel() {
#else
#define DEBUGP(prefix, ...) \
{}
+#define DPSET(Type) false
+#define DPIF(Type, ...) \
+ { \
+ }
#define DP(...) \
{}
#define REPORT(...) FAILURE_MESSAGE(__VA_ARGS__);
#endif // OMPTARGET_DEBUG
+#ifdef OMPTARGET_DEBUG
+// Convert `OpenMPInfoType` to corresponding `DebugInfoType`
+inline bool debugInfoEnabled(OpenMPInfoType InfoType) {
+ switch (InfoType) {
+ case OMP_INFOTYPE_KERNEL_ARGS:
+ [[fallthrough]];
+ case OMP_INFOTYPE_PLUGIN_KERNEL:
+ return DPSET(KERNEL);
+ case OMP_INFOTYPE_MAPPING_EXISTS:
+ [[fallthrough]];
+ case OMP_INFOTYPE_DUMP_TABLE:
+ [[fallthrough]];
+ case OMP_INFOTYPE_MAPPING_CHANGED:
+ [[fallthrough]];
+ case OMP_INFOTYPE_EMPTY_MAPPING:
+ return DPSET(MAP);
+ case OMP_INFOTYPE_DATA_TRANSFER:
+ return DPSET(COPY);
+ case OMP_INFOTYPE_ALL:
+ return DPSET(ALL);
+ }
+}
+#else
+#define debugInfoEnabled(InfoType) false
+#endif // OMPTARGET_DEBUG
+
/// Emit a message giving the user extra information about the runtime if
#define INFO(_flags, _id, ...) \
do { \
- if (getDebugLevel() > 0) { \
+ if (debugInfoEnabled(_flags)) { \
DEBUGP(DEBUG_PREFIX, __VA_ARGS__); \
} else if (getInfoLevel() & _flags) { \
INFO_MESSAGE(_id, __VA_ARGS__); \
diff --git a/offload/include/Shared/EnvironmentVar.h b/offload/include/Shared/EnvironmentVar.h
index 82f434e91a85b..94974615a05d4 100644
--- a/offload/include/Shared/EnvironmentVar.h
+++ b/offload/include/Shared/EnvironmentVar.h
@@ -61,7 +61,8 @@ template <typename Ty> class Envar {
IsPresent = StringParser::parse<Ty>(EnvStr, Data);
if (!IsPresent) {
- DP("Ignoring invalid value %s for envar %s\n", EnvStr, Name.data());
+ DPIF(RTL, "Ignoring invalid value %s for envar %s\n", EnvStr,
+ Name.data());
Data = Default;
}
}
@@ -180,12 +181,13 @@ inline llvm::Error Envar<Ty>::init(llvm::StringRef Name, GetterFunctor Getter,
// not present and reset to the getter value (default).
IsPresent = false;
Data = Default;
- DP("Setter of envar %s failed, resetting to %s\n", Name.data(),
- std::to_string(Data).data());
+ DPIF(RTL, "Setter of envar %s failed, resetting to %s\n", Name.data(),
+ std::to_string(Data).data());
consumeError(std::move(Err));
}
} else {
- DP("Ignoring invalid value %s for envar %s\n", EnvStr, Name.data());
+ DPIF(RTL, "Ignoring invalid value %s for envar %s\n", EnvStr,
+ Name.data());
Data = Default;
}
} else {
diff --git a/offload/libomptarget/LegacyAPI.cpp b/offload/libomptarget/LegacyAPI.cpp
index 033d7a3ef712a..64297d92e879a 100644
--- a/offload/libomptarget/LegacyAPI.cpp
+++ b/offload/libomptarget/LegacyAPI.cpp
@@ -180,7 +180,8 @@ EXTERN int __tgt_target_teams_nowait_mapper(
EXTERN void __kmpc_push_target_tripcount_mapper(ident_t *Loc, int64_t DeviceId,
uint64_t LoopTripcount) {
TIMESCOPE_WITH_IDENT(Loc);
- DP("WARNING: __kmpc_push_target_tripcount has been deprecated and is a noop");
+ DPIF(RTL, "WARNING: __kmpc_push_target_tripcount has been deprecated and is "
+ "a noop");
}
EXTERN void __kmpc_push_target_tripcount(int64_t DeviceId,
diff --git a/offload/libomptarget/OffloadRTL.cpp b/offload/libomptarget/OffloadRTL.cpp
index 04bd21ec91a49..48e0347e8af00 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");
+ DPIF(RTL, "Init offload library!\n");
#ifdef OMPT_SUPPORT
// Initialize OMPT first
llvm::omp::target::ompt::connectLibrary();
@@ -54,12 +54,12 @@ void deinitRuntime() {
assert(PM && "Runtime not initialized");
if (RefCount == 1) {
- DP("Deinit offload library!\n");
+ DPIF(RTL, "Deinit offload library!\n");
// RTL deinitialization has started
RTLAlive = false;
while (RTLOngoingSyncs > 0) {
- DP("Waiting for ongoing syncs to finish, count: %d\n",
- RTLOngoingSyncs.load());
+ DPIF(RTL, "Waiting for ongoing syncs to finish, count: %d\n",
+ RTLOngoingSyncs.load());
std::this_thread::sleep_for(std::chrono::milliseconds(100));
}
PM->deinit();
diff --git a/offload/libomptarget/OpenMP/API.cpp b/offload/libomptarget/OpenMP/API.cpp
index b0f0573833713..14f20f044be89 100644
--- a/offload/libomptarget/OpenMP/API.cpp
+++ b/offload/libomptarget/OpenMP/API.cpp
@@ -76,7 +76,7 @@ EXTERN int omp_get_num_devices(void) {
OMPT_IF_BUILT(ReturnAddressSetterRAII RA(__builtin_return_address(0)));
size_t NumDevices = PM->getNumDevices();
- DP("Call to omp_get_num_devices returning %zd\n", NumDevices);
+ DPIF(DEVICE, "Call to omp_get_num_devices returning %zd\n", NumDevices);
return NumDevices;
}
@@ -86,7 +86,7 @@ EXTERN int omp_get_device_num(void) {
OMPT_IF_BUILT(ReturnAddressSetterRAII RA(__builtin_return_address(0)));
int HostDevice = omp_get_initial_device();
- DP("Call to omp_get_device_num returning %d\n", HostDevice);
+ DPIF(DEVICE, "Call to omp_get_device_num returning %d\n", HostDevice);
return HostDevice;
}
@@ -95,7 +95,7 @@ EXTERN int omp_get_initial_device(void) {
TIMESCOPE();
OMPT_IF_BUILT(ReturnAddressSetterRAII RA(__builtin_return_address(0)));
int HostDevice = omp_get_num_devices();
- DP("Call to omp_get_initial_device returning %d\n", HostDevice);
+ DPIF(DEVICE, "Call to omp_get_initial_device returning %d\n", HostDevice);
return HostDevice;
}
@@ -166,16 +166,17 @@ EXTERN void llvm_omp_target_unlock_mem(void *Ptr, int DeviceNum) {
EXTERN int omp_target_is_present(const void *Ptr, int DeviceNum) {
TIMESCOPE();
OMPT_IF_BUILT(ReturnAddressSetterRAII RA(__builtin_return_address(0)));
- DP("Call to omp_target_is_present for device %d and address " DPxMOD "\n",
- DeviceNum, DPxPTR(Ptr));
+ DPIF(MAP,
+ "Call to omp_target_is_present for device %d and address " DPxMOD "\n",
+ DeviceNum, DPxPTR(Ptr));
if (!Ptr) {
- DP("Call to omp_target_is_present with NULL ptr, returning false\n");
+ DPIF(MAP, "Call to omp_target_is_present with NULL ptr, returning false\n");
return false;
}
if (DeviceNum == omp_get_initial_device()) {
- DP("Call to omp_target_is_present on host, returning true\n");
+ DPIF(MAP, "Call to omp_target_is_present on host, returning true\n");
return true;
}
@@ -192,7 +193,7 @@ EXTERN int omp_target_is_present(const void *Ptr, int DeviceNum) {
/*UpdateRefCount=*/false,
/*UseHoldRefCount=*/false);
int Rc = TPR.isPresent();
- DP("Call to omp_target_is_present returns %d\n", Rc);
+ DPIF(MAP, "Call to omp_target_is_present returns %d\n", Rc);
return Rc;
}
@@ -203,15 +204,16 @@ EXTERN int omp_target_memcpy(void *Dst, const void *Src, size_t Length,
";src_dev=" + std::to_string(SrcDevice) +
";size=" + std::to_string(Length));
OMPT_IF_BUILT(ReturnAddressSetterRAII RA(__builtin_return_address(0)));
- DP("Call to omp_target_memcpy, dst device %d, src device %d, "
- "dst addr " DPxMOD ", src addr " DPxMOD ", dst offset %zu, "
- "src offset %zu, length %zu\n",
- DstDevice, SrcDevice, DPxPTR(Dst), DPxPTR(Src), DstOffset, SrcOffset,
- Length);
+ DPIF(COPY,
+ "Call to omp_target_memcpy, dst device %d, src device %d, "
+ "dst addr " DPxMOD ", src addr " DPxMOD ", dst offset %zu, "
+ "src offset %zu, length %zu\n",
+ DstDevice, SrcDevice, DPxPTR(Dst), DPxPTR(Src), DstOffset, SrcOffset,
+ Length);
if (!Dst || !Src || Length <= 0) {
if (Length == 0) {
- DP("Call to omp_target_memcpy with zero length, nothing to do\n");
+ DPIF(COPY, "Call to omp_target_memcpy with zero length, nothing to do\n");
return OFFLOAD_SUCCESS;
}
@@ -225,12 +227,12 @@ EXTERN int omp_target_memcpy(void *Dst, const void *Src, size_t Length,
if (SrcDevice == omp_get_initial_device() &&
DstDevice == omp_get_initial_device()) {
- DP("copy from host to host\n");
+ DPIF(COPY, "copy from host to host\n");
const void *P = memcpy(DstAddr, SrcAddr, Length);
if (P == NULL)
Rc = OFFLOAD_FAIL;
} else if (SrcDevice == omp_get_initial_device()) {
- DP("copy from host to device\n");
+ DPIF(COPY, "copy from host to device\n");
auto DstDeviceOrErr = PM->getDevice(DstDevice);
if (!DstDeviceOrErr)
FATAL_MESSAGE(DstDevice, "%s",
@@ -238,7 +240,7 @@ EXTERN int omp_target_memcpy(void *Dst, const void *Src, size_t Length,
AsyncInfoTy AsyncInfo(*DstDeviceOrErr);
Rc = DstDeviceOrErr->submitData(DstAddr, SrcAddr, Length, AsyncInfo);
} else if (DstDevice == omp_get_initial_device()) {
- DP("copy from device to host\n");
+ DPIF(COPY, "copy from device to host\n");
auto SrcDeviceOrErr = PM->getDevice(SrcDevice);
if (!SrcDeviceOrErr)
FATAL_MESSAGE(SrcDevice, "%s",
@@ -246,7 +248,7 @@ EXTERN int omp_target_memcpy(void *Dst, const void *Src, size_t Length,
AsyncInfoTy AsyncInfo(*SrcDeviceOrErr);
Rc = SrcDeviceOrErr->retrieveData(DstAddr, SrcAddr, Length, AsyncInfo);
} else {
- DP("copy from device to device\n");
+ DPIF(COPY, "copy from device to device\n");
auto SrcDeviceOrErr = PM->getDevice(SrcDevice);
if (!SrcDeviceOrErr)
FATAL_MESSAGE(SrcDevice, "%s",
@@ -278,7 +280,7 @@ EXTERN int omp_target_memcpy(void *Dst, const void *Src, size_t Length,
free(Buffer);
}
- DP("omp_target_memcpy returns %d\n", Rc);
+ DPIF(COPY, "omp_target_memcpy returns %d\n", Rc);
return Rc;
}
@@ -301,12 +303,12 @@ static int libomp_target_memcpy_async_task(int32_t Gtid, kmp_task_t *Task) {
Args->DstOffsets, Args->SrcOffsets, Args->DstDimensions,
Args->SrcDimensions, Args->DstDevice, Args->SrcDevice);
- DP("omp_target_memcpy_rect returns %d\n", Rc);
+ DPIF(COPY, "omp_target_memcpy_rect returns %d\n", Rc);
} else {
Rc = omp_target_memcpy(Args->Dst, Args->Src, Args->Length, Args->DstOffset,
Args->SrcOffset, Args->DstDevice, Args->SrcDevice);
- DP("omp_target_memcpy returns %d\n", Rc);
+ DPIF(COPY, "omp_target_memcpy returns %d\n", Rc);
}
// Release the arguments object
@@ -380,8 +382,9 @@ EXTERN void *omp_target_memset(void *Ptr, int ByteVal, size_t NumBytes,
int DeviceNum) {
TIMESCOPE();
OMPT_IF_BUILT(ReturnAddressSetterRAII RA(__builtin_return_address(0)));
- DP("Call to omp_target_memset, device %d, device pointer %p, size %zu\n",
- DeviceNum, Ptr, NumBytes);
+ DPIF(COPY,
+ "Call to omp_target_memset, device %d, device pointer %p, size %zu\n",
+ DeviceNum, Ptr, NumBytes);
// Behave as a no-op if N==0 or if Ptr is nullptr (as a useful implementation
// of unspecified behavior, see OpenMP spec).
@@ -390,7 +393,7 @@ EXTERN void *omp_target_memset(void *Ptr, int ByteVal, size_t NumBytes,
}
if (DeviceNum == omp_get_initial_device()) {
- DP("filling memory on host via memset");
+ DPIF(COPY, "filling memory on host via memset");
memset(Ptr, ByteVal, NumBytes); // ignore return value, memset() cannot fail
} else {
// TODO: replace the omp_target_memset() slow path with the fast path.
@@ -410,12 +413,12 @@ EXTERN void *omp_target_memset(void *Ptr, int ByteVal, size_t NumBytes,
// If the omp_target_alloc has failed, let's just not do anything.
// omp_target_memset does not have any good way to fail, so we
// simply avoid a catastrophic failure of the process for now.
- DP("omp_target_memset failed to fill memory due to error with "
- "omp_target_alloc");
+ DPIF(COPY, "omp_target_memset failed to fill memory due to error with "
+ "omp_target_alloc");
}
}
- DP("omp_target_memset returns %p\n", Ptr);
+ DPIF(COPY, "omp_target_memset returns %p\n", Ptr);
return Ptr;
}
@@ -423,8 +426,10 @@ EXTERN void *omp_target_memset_async(void *Ptr, int ByteVal, size_t NumBytes,
int DeviceNum, int DepObjCount,
omp_depend_t...
[truncated]
|
There was a problem hiding this comment.
Honestly I would prefer this to be completely reworked to be more similar to how LLVM handles its debugging. This was mostly a holdover from before we just linked against LLVM for the runtime. I had a patch to do that years ago but never finished and I lost the diff.
|
+1 to make it more like what LLVM is doing. |
|
I'd really appreciate help updating the debugging if you're up for it, it's been a thorn in my side for awhile. I'd recommend doing it separately for the plugins / libomptarget to keep the patches more understandable. |
Any pointers on that? |
https://github.com/llvm/llvm-project/blob/2d23a60de866f2f858e54e1c87ec5a7948b82079/llvm/lib/Support/Debug.cpp Same concept but we should enable it with environment variables or some other global config instead of command line flag. |
| OptVal.Level = std::atoi(EnvStr); | ||
| if (OptVal.Level) | ||
| return OptVal; // defined as numeric value | ||
| struct DebugStrToBitTy { |
So do you mean to have something that follows the same kind of style or to actually use the code in llvm/support? Because I think the later can be a bit trickier given that in its current state it will probably require the build mode of llvm and libomptarget to match. |
No, not directly using Support since it uses different activation methods, but we'd probably want to handle it the same with |
Ok, I think I can try prototype something simple along those lines we can get sure we all agree before making widespread changes. |
|
I put together a bit of code. Messages are constructed in C++ stream fashion can be tagged with a channel and level. So the debug messages code could look something like this: Then configuration through LIBOMPTARGET_DEBUG could be something like this: is this the general idea that you were describing @jhuber6 ? |
|
We should copy the OFFLOAD_DEBUG(TAG, dbgs() << "foo");We could make a default tag if you were so inclined, doable with some macro magic like #define GET_MACRO(1, 2, NAME, ...) NAME
GET_MACRO(__VA_ARGS__, FORM1, FORM2)(a, b, c) |
Current llvm/support has support for levels that mostly why I added them. While I'd agree that tags/kinds are more important, I think levels are still quite useful to avoid "debug pollution". Of course, you could still create different tags (e.g. X_0, X_1, ...) but that's just way more inconvenient to use imo. Note that libomp also has levels for debug.
This I disagree quite a bit. I don't see any benefit to having to explicitly state "dbgs" everytime, while it makes significantly harder to be able to prefix information to every message. I think that's something that we should have to be able to print not just the current "target_name" as we currently do, but also other important information like pid (very important for MPI programs), thread id, ... What I have right now is the "main" macro that you don't need to specify it and a second "low-level" one that allows to fully specify everything for those rare cases where that's really necessary (and I'd agree there will be some).
I don't care very much about having a default tag tbh :) My current code is like those macros because I thought it might be nice to have it if you just want to print a message and don't think too much about it but it's no a big deal if not. |
6 participants
There are some users who use debug build and/or enable debug printing but are only interested in a subset of the emitted messages. This change addresses such needs by extending the existing environment variable
LIBOMPTARGET_DEBUGwhich accepts both numeric and string values that represent subsets of the printed message. The behavior when it is a numeric value did not change.