Skip to content

Commit ae136f4

Browse files
frasercrmckcallumfare
authored andcommitted
[HIP] Improve urEnqueueUSMAdvise return code for non-shared mem
We can only call hipMemAdvise on managed pointers allocated via hipMallocManaged, so can only support the UR entry point on pointers allocated via urUSMSharedAlloc. When faced with other pointers, we're supposed to ignore them and result "success", but instead this PR continues the tradition of returning UR_RESULT_ERROR_ADAPTER_SPECIFIC with a warning message providing the user more information. This should all be fixed up later when there's a better warning mechanism.
1 parent 2084114 commit ae136f4

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

source/adapters/hip/enqueue.cpp

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,8 +1424,6 @@ urEnqueueUSMAdvise(ur_queue_handle_t hQueue, const void *pMem, size_t size,
14241424
UR_ASSERT(size <= PointerRangeSize, UR_RESULT_ERROR_INVALID_SIZE);
14251425
#endif
14261426

1427-
ur_result_t Result = UR_RESULT_SUCCESS;
1428-
14291427
try {
14301428
ScopedContext Active(Device);
14311429
std::unique_ptr<ur_event_handle_t_> EventPtr{nullptr};
@@ -1480,6 +1478,20 @@ urEnqueueUSMAdvise(ur_queue_handle_t hQueue, const void *pMem, size_t size,
14801478
// UR_USM_MEM_ADVICE_SET/MEM_ADVICE_CLEAR_READ_MOSTLY.
14811479
}
14821480

1481+
// hipMemAdvise only supports managed memory allocated via
1482+
// hipMallocManaged. We can't support this API with any other types of
1483+
// pointer. We should ignore them and result UR_RESULT_SUCCESS but instead
1484+
// we report a warning.
1485+
// FIXME: Fix this up when there's a better warning mechanism.
1486+
if (auto ptrAttribs = getPointerAttributes(pMem);
1487+
!ptrAttribs || !ptrAttribs->isManaged) {
1488+
releaseEvent();
1489+
setErrorMessage("mem_advise is ignored as the pointer argument is not "
1490+
"a shared USM pointer",
1491+
UR_RESULT_SUCCESS);
1492+
return UR_RESULT_ERROR_ADAPTER_SPECIFIC;
1493+
}
1494+
14831495
const auto DeviceID = Device->get();
14841496
if (advice & UR_USM_ADVICE_FLAG_DEFAULT) {
14851497
UR_CHECK_ERROR(
@@ -1493,7 +1505,11 @@ urEnqueueUSMAdvise(ur_queue_handle_t hQueue, const void *pMem, size_t size,
14931505
hipMemAdvise(pMem, size, hipMemAdviseUnsetCoarseGrain, DeviceID));
14941506
#endif
14951507
} else {
1496-
Result = setHipMemAdvise(HIPDevicePtr, size, advice, DeviceID);
1508+
ur_result_t Result =
1509+
setHipMemAdvise(HIPDevicePtr, size, advice, DeviceID);
1510+
assert((Result == UR_RESULT_SUCCESS ||
1511+
Result == UR_RESULT_ERROR_INVALID_ENUMERATION) &&
1512+
"Unexpected return code");
14971513
// UR_RESULT_ERROR_INVALID_ENUMERATION is returned when using a valid
14981514
// but currently unmapped advice arguments as not supported by this
14991515
// platform. Therefore, warn the user instead of throwing and aborting
@@ -1509,12 +1525,12 @@ urEnqueueUSMAdvise(ur_queue_handle_t hQueue, const void *pMem, size_t size,
15091525

15101526
releaseEvent();
15111527
} catch (ur_result_t err) {
1512-
Result = err;
1528+
return err;
15131529
} catch (...) {
1514-
Result = UR_RESULT_ERROR_UNKNOWN;
1530+
return UR_RESULT_ERROR_UNKNOWN;
15151531
}
15161532

1517-
return Result;
1533+
return UR_RESULT_SUCCESS;
15181534
}
15191535

15201536
UR_APIEXPORT ur_result_t UR_APICALL urEnqueueUSMFill2D(

test/conformance/enqueue/enqueue_adapter_hip.match

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,15 @@ urEnqueueKernelLaunchUSMLinkedList.Success/AMD_HIP_BACKEND___{{.*}}___UsePoolEna
77
{{OPT}}urEnqueueMemBufferCopyRectTestWithParam.Success/AMD_HIP_BACKEND___{{.*}}___copy_3d_2d
88
{{OPT}}urEnqueueMemBufferWriteRectTestWithParam.Success/AMD_HIP_BACKEND___{{.*}}___write_row_2D
99
{{OPT}}urEnqueueMemBufferWriteRectTestWithParam.Success/AMD_HIP_BACKEND___{{.*}}___write_3d_2d
10+
11+
# HIP doesn't ignore unsupported USM advice or prefetching. Instead of
12+
# returning UR_RESULT_SUCCESS as per the spec, it instead returns
13+
# UR_RESULT_ERROR_ADAPTER_SPECIFIC to issue a warning. These tests will fail
14+
# until this is rectified.
1015
urEnqueueUSMAdviseWithParamTest.Success/AMD_HIP_BACKEND___{{.*}}___UR_USM_ADVICE_FLAG_DEFAULT
1116
urEnqueueUSMAdviseTest.MultipleParamsSuccess/AMD_HIP_BACKEND___{{.*}}_
12-
urEnqueueUSMAdviseTest.NonCoherentDeviceMemorySuccessOrWarning/AMD_HIP_BACKEND___{{.*}}_
1317
urEnqueueUSMPrefetchWithParamTest.Success/AMD_HIP_BACKEND___{{.*}}___UR_USM_MIGRATION_FLAG_DEFAULT
1418
urEnqueueUSMPrefetchWithParamTest.CheckWaitEvent/AMD_HIP_BACKEND___{{.*}}___UR_USM_MIGRATION_FLAG_DEFAULT
19+
1520
urEnqueueTimestampRecordingExpTest.Success/AMD_HIP_BACKEND___{{.*}}
1621
urEnqueueTimestampRecordingExpTest.SuccessBlocking/AMD_HIP_BACKEND___{{.*}}

0 commit comments

Comments
 (0)