Skip to content

[SYCL][HIP][L0][NATIVECPU] urAdapterGetLastError fix so messages aren't lost. #10953

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sycl/plugins/unified_runtime/ur/adapters/hip/adapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urAdapterRetain(ur_adapter_handle_t) {

UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetLastError(
ur_adapter_handle_t, const char **ppMessage, int32_t *pError) {
std::ignore = pError;
*ppMessage = ErrorMessage;
*pError = ErrorMessageCode;
return UR_RESULT_SUCCESS;
return ErrorMessageCode;
}

UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetInfo(ur_adapter_handle_t,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetLastError(
///< error code will be stored.
) {
std::ignore = Adapter;
std::ignore = Message;
std::ignore = Error;
urPrint("[UR][L0] %s function not implemented!\n", __FUNCTION__);
return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
*Message = ErrorMessage;
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks @JackAKirk . L0 implementation for getLastError is being done here #10784. There's a discussion here on how it should behave. i'm planning on following on that, so feel free to address the hip and cpu adapters only on this patch, or to address also L0 based on those comments. Either way works for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, L0 impl here is already consistent with those comments (all backends have the same impl).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jandres742 do you want me to remove the l0 implementation from this PR, or are you happy with it?

Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

@JackAKirk : implementation here is good, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK thanks!

return ErrorMessageCode;
}

UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetInfo(ur_adapter_handle_t,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueDeviceGlobalVariableWrite(
if (GlobalVarSize < Offset + Count) {
setErrorMessage("Write device global variable is out of range.",
UR_RESULT_ERROR_INVALID_VALUE);
return UR_RESULT_ERROR_UNKNOWN;
return UR_RESULT_ERROR_ADAPTER_SPECIFIC;
}

// Copy engine is preferred only for host to device transfer.
Expand Down Expand Up @@ -332,7 +332,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueDeviceGlobalVariableRead(
if (GlobalVarSize < Offset + Count) {
setErrorMessage("Read from device global variable is out of range.",
UR_RESULT_ERROR_INVALID_VALUE);
return UR_RESULT_ERROR_UNKNOWN;
return UR_RESULT_ERROR_ADAPTER_SPECIFIC;
}

// Copy engine is preferred only for host to device transfer.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urAdapterRetain(ur_adapter_handle_t) {

UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetLastError(
ur_adapter_handle_t, const char **ppMessage, int32_t *pError) {
std::ignore = pError;
*ppMessage = ErrorMessage;
*pError = ErrorMessageCode;
return UR_RESULT_SUCCESS;
return ErrorMessageCode;
}

UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetInfo(ur_adapter_handle_t,
Expand Down