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

Adapter-specific tracing interferes with regular tracing output #2002

Open
callumfare opened this issue Aug 22, 2024 · 6 comments · May be fixed by #2101
Open

Adapter-specific tracing interferes with regular tracing output #2002

callumfare opened this issue Aug 22, 2024 · 6 comments · May be fixed by #2101

Comments

@callumfare
Copy link
Contributor

When tracing is enabled via sycl using the SYCL_UR_TRACE environment variable, some adapters also enable tracing of internal UR calls (see here).

Regular UR tracing works by printing the function call (--->urDeviceFoo) before calling into the adapter, and the arguments after the adapter returns ((ur_device_handle_t hFoo, int Bar) -> UR_RESULT_SUCCESS). When the UR adapter also prints, it becomes interleaved in this output, making it hard to read and test against.

This was ok when we still used PI tracing as PI put the function name and each argument on separate lines, but UR tracing outputs everything on a single line.

This has resulted in some SYCL E2E requiring awkward rewrites or bypassing adapter tracing altogether, e.g. intel/llvm#15167

Possible fixes include:

  • Update ur_print.hpp to print over multiple lines like PI did
  • Don't enable adapter-specific printing by default (has the downside of internal UR calls being invisible)
  • Add an option to suppress adapter-specific printing for some E2E tests to use
@kbenzie
Copy link
Contributor

kbenzie commented Aug 22, 2024

Don't enable adapter-specific printing by default (has the downside of internal UR calls being invisible)

This touches on another issue @nrspruit noticed, there are also hidden UR calls in the validation layer which don't get picked up by tracing.

@pbalcer
Copy link
Contributor

pbalcer commented Sep 2, 2024

Don't enable adapter-specific printing by default (has the downside of internal UR calls being invisible)

This touches on another issue @nrspruit noticed, there are also hidden UR calls in the validation layer which don't get picked up by tracing.

I think this will be resolved by #2028

@pbalcer
Copy link
Contributor

pbalcer commented Sep 2, 2024

I never liked how PI interleaved the output of these operations.

If this is within the scope of a single logger instance, we might also consider adding call nesting/indent option to the logger, like ltrace --indent option.
This way, the output could be something like this:

urEnqueueKernel(...)
    zeCommandListAppendLaunchKernel() -> ZE_SUCCESS;
    ...
-> UR_SUCCESS

@lslusarczyk
Copy link
Contributor

lslusarczyk commented Sep 17, 2024

I noticed that regular UR tracing writes logs to stdout while adapter writes to stderr.
These can be interleaved in any way without our control therefore I want to unify by changing regular UR tracing to write to stderr too.
@pbalcer , @callumfare , @kbenzie , do you have objections on this proposed change?

@pbalcer
Copy link
Contributor

pbalcer commented Sep 18, 2024

By default, logger outputs to stderr: https://github.com/oneapi-src/unified-runtime/blob/main/source/common/logger/ur_logger.hpp#L134 if not configured differently.

SYCL_UR_TRACE may be setting output to stdout to preserve backward compatibility. I imagine there might be scripts that assume that the output is printed to stdout.

In general, libraries should print logs to stderr.

@kbenzie
Copy link
Contributor

kbenzie commented Sep 18, 2024

I'm fine with unifying on outputting to stderr in principle. It's a good point about backwards compatibility, will need to check how SYCL_PI_TRACE was behaving before it was removed.

@lslusarczyk lslusarczyk linked a pull request Sep 18, 2024 that will close this issue
lslusarczyk added a commit to lslusarczyk/unified-runtime that referenced this issue Sep 19, 2024
Fixed oneapi-src#2002 issue.
Regular UR tracing prints now calls in two separate lines like PI does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants