-
Notifications
You must be signed in to change notification settings - Fork 111
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
fixed mixed output of adapter and regular traces #2101
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really a fan of this approach, I'd prefer to have indents like ltrace. But I guess it solves the immediate problem...
Also, please try to keep the first line of the commit message under 50 characters, and the rest of the message lines under ~70.
$ man git commit
...
Though not required, it’s a good idea to begin the commit message with a single short (less than 50 character)
line summarizing the change, followed by a blank line and then a more thorough description. The text up to the
first blank line in a commit message is treated as the commit title, and that title is used throughout Git.
@@ -40,16 +40,18 @@ __urdlllocal ur_result_t UR_APICALL urAdapterGet( | |||
uint64_t instance = getContext()->notify_begin(UR_FUNCTION_ADAPTER_GET, | |||
"urAdapterGet", ¶ms); | |||
|
|||
getContext()->logger.info("---> urAdapterGet"); | |||
std::ostringstream args_str; | |||
ur::extras::printFunctionParams(args_str, UR_FUNCTION_ADAPTER_GET, ¶ms); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not all the arguments can be accessed here, because some of them may be output arguments, with uninitialized content, which results in UB on access.
We either have to selectively access the parameters, adding an option to printFunctionParams to decide whether we are pre or post function execution, or, we can simply read those arguments after the execution. Which is what the original implementation did, for this exact reason.
30f8e30
to
2d10f85
Compare
makes llvm work with this change: oneapi-src/unified-runtime#2101 fixes this bug: oneapi-src/unified-runtime#2002
makes llvm work with this change: oneapi-src/unified-runtime#2101 fixes this bug: oneapi-src/unified-runtime#2002
makes llvm work with this change: oneapi-src/unified-runtime#2101 fixes this bug: oneapi-src/unified-runtime#2002
makes llvm work with this change: oneapi-src/unified-runtime#2101 fixes this bug: oneapi-src/unified-runtime#2002
Fixed oneapi-src#2002 issue. Regular UR tracing prints now calls in two separate lines like PI does.
2b5d716
to
12e0cee
Compare
makes llvm work with this change: oneapi-src/unified-runtime#2101 fixes this bug: oneapi-src/unified-runtime#2002
makes llvm work with this change: oneapi-src/unified-runtime#2101 fixes this bug: oneapi-src/unified-runtime#2002
Fixed #2002 issue.
Regular UR tracing prints calls in two separate lines like PI does.