Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GPU] Logging cleanup #2446

Merged
merged 5 commits into from
Jan 30, 2025
Merged

[GPU] Logging cleanup #2446

merged 5 commits into from
Jan 30, 2025

Conversation

echeresh
Copy link
Contributor

@echeresh echeresh commented Jan 18, 2025

Jira: https://jira.devtools.intel.com/browse/MFDNN-11400.

List of changes:

  • Moved logging functionality to src/gpu/intel/logging.hpp with some slight refactoring (constant -> enum class)
    • Renamed ir_{warning,perf,trace,...} to gpu-prefixed versions
  • Reworked logging format: see src/gpu/intel/logging.hpp
    • Example: [TRACE][ codegen.cpp:95] codegen:bind h_34 -> r18 - r19
    • Added level tag and file name/line number information
  • Simplified logging implementation, mostly minor things:
    • Removed ir_check related not-relly-important features as base_logger_t, fatal and dynamic log levels, IR check guard
    • Added automatic new-line after every log message
  • Dropped ir_assert (and similar macros), renamed usages to the existing gpu_assert, etc

Things left for the future:

  • Documentation
  • New log level: debug (or maybe refactored info/trace combination). Now "trace" is used for both very detailed tracing and some useful/more high-level logging. There should be more granularity, e.g.:
    • Trace: max detail level, logging after every IR pass and even from inside of some passes
    • Debug/info: less details but enough to understand what kernel looks like, e.g. kernel parameters + final IR

@echeresh echeresh added the platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel label Jan 18, 2025
@echeresh echeresh requested a review from a team as a code owner January 18, 2025 01:10
@rjoursler
Copy link
Contributor

rjoursler commented Jan 18, 2025

Simplifying log levels, e.g. drop perf-specific levels as they are not really used AFAIK (?)

I have often used this when trying to optimize IR creation time. The main issue is that the trace level introduces overhead (~30% from what I remember) and this overhead was not evenly distributed. This makes is so that performance in trace mode only loosely corresponds to performance in release mode when prioritizing optimizations. Could we add perf information (perhaps in a more compressed format) to the info level, analogous to how oneDNN verbose maps into spdlog levels?

@echeresh echeresh force-pushed the echeresh/log-cleanup branch 2 times, most recently from 3fe409d to 8b00fb1 Compare January 21, 2025 21:33
@echeresh echeresh changed the title WIP: [GPU] Logging cleanup [GPU] Logging cleanup Jan 21, 2025
@echeresh
Copy link
Contributor Author

Simplifying log levels, e.g. drop perf-specific levels as they are not really used AFAIK (?)

I have often used this when trying to optimize IR creation time. The main issue is that the trace level introduces overhead (~30% from what I remember) and this overhead was not evenly distributed. This makes is so that performance in trace mode only loosely corresponds to performance in release mode when prioritizing optimizations. Could we add perf information (perhaps in a more compressed format) to the info level, analogous to how oneDNN verbose maps into spdlog levels?

Thanks. Yes, combining it with info might be a good option. Let's keep it as is for now. I plan to open one more PR, will think about this later.

src/gpu/intel/jit/utils/utils.hpp Outdated Show resolved Hide resolved
src/gpu/intel/logging.hpp Outdated Show resolved Hide resolved
src/gpu/intel/logging.hpp Outdated Show resolved Hide resolved
src/gpu/intel/jit/utils/utils.hpp Outdated Show resolved Hide resolved
src/gpu/intel/logging.hpp Outdated Show resolved Hide resolved
@echeresh echeresh force-pushed the echeresh/log-cleanup branch from 8b00fb1 to f08650a Compare January 23, 2025 20:27
src/gpu/intel/jit/reorder/gen_reorder.cpp Outdated Show resolved Hide resolved
src/gpu/intel/jit/pooling/gen_pooling.cpp Outdated Show resolved Hide resolved
@echeresh echeresh force-pushed the echeresh/log-cleanup branch from f08650a to 34ae4c7 Compare January 30, 2025 01:14
@echeresh echeresh force-pushed the echeresh/log-cleanup branch from 34ae4c7 to 4c4a607 Compare January 30, 2025 19:10
@echeresh
Copy link
Contributor Author

make test
set test_scope=NIGHTLY
disable test_device_cpu
enable arch_gpu_xe-hpc
enable arch_gpu_xe-hpg-atsm
enable arch_gpu_xe-hpg-dg2
enable arch_gpu_xe-lp
enable arch_gpu_xe-lpg
enable arch_gpu_xe-lpg+
enable arch_gpu_xe2-hpg-bmg
enable arch_gpu_xe2-lpg
enable arch_gpu_xe3-lpg

@echeresh echeresh merged commit 4727d3c into main Jan 30, 2025
4 of 5 checks passed
@echeresh echeresh deleted the echeresh/log-cleanup branch January 30, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants