This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Remove macro guards for device debugging
ClosedPublic

Authored by jhuber6 on Oct 19 2021, 9:14 AM.

Details

Summary

The plugin currently uses a macro to check if this is a debug built
before assigning the debug kind variable to the device environment
struct. This is being deprecated because the new device runtime does not
maintain separate debug builds and should always be availible.

Diff Detail

Event Timeline

jhuber6 created this revision.Oct 19 2021, 9:14 AM
jhuber6 requested review of this revision.Oct 19 2021, 9:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 19 2021, 9:14 AM
tianshilei1992 accepted this revision.Oct 19 2021, 9:15 AM

Why not. :-)

This revision is now accepted and ready to land.Oct 19 2021, 9:15 AM
This revision was automatically updated to reflect the committed changes.

Seems inconsistent. Could we leave the debug output available in release builds across the host plugins? Measured in the noise for performance on rocm (branches are easily predicted) and it's useful to toggle debug output on without recompiling.

Seems inconsistent. Could we leave the debug output available in release builds across the host plugins? Measured in the noise for performance on rocm (branches are easily predicted) and it's useful to toggle debug output on without recompiling.

This is specifically for toggling debugging using the new device runtime, which will be the default very soon. The new device runtime has no concept of a debug version, and is instead configured at compile-time by the user by specifying -fopenmp-target-debug=<N>. At runtime the user then specifies LIBOMPTARGET_DEVICE_RTL_DEBUG=<N> to activate those features. Having the actual use of this being guarded by a separate debug build defeats the purpose at least on the device side.

I really like the scheme of choosing debug level at application compile time, instead of at compiler runtime build time. I'm suggesting we run with that idea for the host runtime, effectively building the print expansion of DP(...) into the release build instead of eliding it.

We want to get rid of OMPTARGET_DEBUG as much as possible, which probably means completely. This should have never been guarded. LGTM