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

src: handle DNNL_VERBOSE=OFF support through an early exit (fixes MFDNN-12862) #2300

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dzarukin
Copy link
Contributor

Fixes MFDNN-12862

Disabled verbose shouldn't print data even if requested. The change is complied with this requirement with a marginal price of the library size for disabled verbose build, but easier way to support this mode.

Disabled verbose shouldn't print data even if requested. The change is
complied with this requirement with a marginal price of the library size
for disabled verbose build, but easier way to support this mode.
@dzarukin dzarukin requested review from a team as code owners December 20, 2024 02:45
@github-actions github-actions bot added platform:cpu-x64 Intel64/AMD64 processors. Codeowner: @oneapi-src/onednn-cpu-x64 platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel component:graph-api Codeowner: @oneapi-src/onednn-graph labels Dec 20, 2024
@dzarukin
Copy link
Contributor Author

make test

return std::string();
}

void verbose_printf_impl(const char *raw_fmt_str, verbose_t::flag_kind kind) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify why these were not enough? At a glance, it seems that under DISABLE_VERBOSE, the behavior after the patch should be similar (return empty string or empty function).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number of functions was growing, they are used across different places in verbose-reporting functionality. I decided that instead of handling each function under if (disabled) section (including future added) separately, it's just easier to force some of them to return empty strings under that build flag and call it a day. Should be no issues with future functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:graph-api Codeowner: @oneapi-src/onednn-graph platform:cpu-x64 Intel64/AMD64 processors. Codeowner: @oneapi-src/onednn-cpu-x64 platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants