Skip to content

Commit

Permalink
Address feedback - missing breaks in switches, more appropriate error…
Browse files Browse the repository at this point in the history
… codes or replacing original

error message to use an assert from cassert header and some other minor fixes.
  • Loading branch information
martygrant committed Jan 11, 2024
1 parent 2b2bb12 commit ae9ae1e
Show file tree
Hide file tree
Showing 21 changed files with 62 additions and 90 deletions.
2 changes: 1 addition & 1 deletion source/adapters/cuda/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include <ur/ur.hpp>

/**
* Call an UR API and, if the result is not UR_RESULT_SUCCESS, automatically
* Call a UR API and, if the result is not UR_RESULT_SUCCESS, automatically
* return from the current function.
*/
#define UR_RETURN_ON_FAILURE(urCall) \
Expand Down
9 changes: 4 additions & 5 deletions source/adapters/cuda/device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
UR_CHECK_ERROR(cuDeviceGetAttribute(
&ECCEnabled, CU_DEVICE_ATTRIBUTE_ECC_ENABLED, hDevice->get()));

if ((ECCEnabled != 0) | (ECCEnabled != 1)) {
if ((ECCEnabled != 0) || (ECCEnabled != 1)) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
auto Result = static_cast<bool>(ECCEnabled);
Expand All @@ -569,7 +569,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
UR_CHECK_ERROR(cuDeviceGetAttribute(
&IsIntegrated, CU_DEVICE_ATTRIBUTE_INTEGRATED, hDevice->get()));

if ((IsIntegrated != 0) | (IsIntegrated != 1)) {
if ((IsIntegrated != 0) || (IsIntegrated != 1)) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
auto result = static_cast<bool>(IsIntegrated);
Expand Down Expand Up @@ -852,9 +852,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
case UR_DEVICE_INFO_GLOBAL_MEM_FREE: {
size_t FreeMemory = 0;
size_t TotalMemory = 0;
if (cuMemGetInfo(&FreeMemory, &TotalMemory) != CUDA_SUCCESS) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(cuMemGetInfo(&FreeMemory, &TotalMemory) != CUDA_SUCCESS &&
"failed cuMemGetInfo() API.");
return ReturnValue(FreeMemory);
}
case UR_DEVICE_INFO_MEMORY_CLOCK_RATE: {
Expand Down
14 changes: 8 additions & 6 deletions source/adapters/cuda/enqueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -844,24 +844,26 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemBufferFill(
}

static ur_result_t imageElementByteSize(CUDA_ARRAY_DESCRIPTOR ArrayDesc,
int *Size) {
unsigned int *Size) {
switch (ArrayDesc.Format) {
case CU_AD_FORMAT_UNSIGNED_INT8:
case CU_AD_FORMAT_SIGNED_INT8:
*Size = 1;
break;
case CU_AD_FORMAT_UNSIGNED_INT16:
case CU_AD_FORMAT_SIGNED_INT16:
case CU_AD_FORMAT_HALF:
*Size = 2;
break;
case CU_AD_FORMAT_UNSIGNED_INT32:
case CU_AD_FORMAT_SIGNED_INT32:
case CU_AD_FORMAT_FLOAT:
*Size = 4;
break;
default:
return UR_RESULT_ERROR_INVALID_IMAGE_SIZE;
return UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR;
*Size = 0;
}
return UR_RESULT_SUCCESS;
}

/// General ND memory copy operation for images (where N > 1).
Expand Down Expand Up @@ -957,7 +959,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageRead(
CUDA_ARRAY_DESCRIPTOR ArrayDesc;
UR_CHECK_ERROR(cuArrayGetDescriptor(&ArrayDesc, Array));

int ElementByteSize = 0;
unsigned int ElementByteSize = 0;
UR_RETURN_ON_FAILURE(imageElementByteSize(ArrayDesc, &ElementByteSize));

size_t ByteOffsetX = origin.x * ElementByteSize * ArrayDesc.NumChannels;
Expand Down Expand Up @@ -1030,7 +1032,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageWrite(
CUDA_ARRAY_DESCRIPTOR ArrayDesc;
UR_CHECK_ERROR(cuArrayGetDescriptor(&ArrayDesc, Array));

int ElementByteSize = 0;
unsigned int ElementByteSize = 0;
UR_RETURN_ON_FAILURE(imageElementByteSize(ArrayDesc, &ElementByteSize));

size_t ByteOffsetX = origin.x * ElementByteSize * ArrayDesc.NumChannels;
Expand Down Expand Up @@ -1110,7 +1112,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageCopy(
UR_ASSERT(SrcArrayDesc.NumChannels == DstArrayDesc.NumChannels,
UR_RESULT_ERROR_INVALID_MEM_OBJECT);

int ElementByteSize = 0;
unsigned int ElementByteSize = 0;
UR_RETURN_ON_FAILURE(imageElementByteSize(SrcArrayDesc, &ElementByteSize));

size_t DstByteOffsetX =
Expand Down
13 changes: 4 additions & 9 deletions source/adapters/cuda/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,7 @@ ur_result_t ur_event_handle_t_::record() {

try {
EventID = Queue->getNextEventID();
if (EventID == 0) {
return UR_RESULT_ERROR_INVALID_EVENT;
}
assert(EventID == 0 && "Unrecoverable program state reached in event identifier overflow.");
UR_CHECK_ERROR(cuEventRecord(EvEnd, Stream));
} catch (ur_result_t error) {
Result = error;
Expand Down Expand Up @@ -238,19 +236,16 @@ urEventWait(uint32_t numEvents, const ur_event_handle_t *phEventWaitList) {
UR_APIEXPORT ur_result_t UR_APICALL urEventRetain(ur_event_handle_t hEvent) {
const auto RefCount = hEvent->incrementReferenceCount();

if (RefCount == 0) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(RefCount == 0 &&
"Reference count overflow detected in urEventRetain.");

return UR_RESULT_SUCCESS;
}

UR_APIEXPORT ur_result_t UR_APICALL urEventRelease(ur_event_handle_t hEvent) {
// double delete or someone is messing with the ref count.
// either way, cannot safely proceed.
if (hEvent->getReferenceCount() == 0) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(hEvent->getReferenceCount() == 0 && "Unrecoverable program state reached in urEventRetain.");

// decrement ref count. If it is 0, delete the event.
if (hEvent->decrementReferenceCount() == 0) {
Expand Down
14 changes: 6 additions & 8 deletions source/adapters/cuda/memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,11 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemRelease(ur_mem_handle_t hMem) {
Result = UR_RESULT_ERROR_OUT_OF_RESOURCES;
}

if (Result != UR_RESULT_SUCCESS) {
// A reported CUDA error is either an implementation or an asynchronous CUDA
// error for which it is unclear if the function that reported it succeeded
// or not. Either way, the state of the program is compromised and likely
// unrecoverable.
return UR_RESULT_ERROR_INVALID_OPERATION;
}
// A reported CUDA error is either an implementation or an asynchronous CUDA
// error for which it is unclear if the function that reported it succeeded
// or not. Either way, the state of the program is compromised and likely
// unrecoverable.
assert(Result != UR_RESULT_SUCCESS && "Unrecoverable program state reached in urMemRelease.");

return UR_RESULT_SUCCESS;
}
Expand Down Expand Up @@ -306,7 +304,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemImageCreate(
PixelTypeSizeBytes = 4;
break;
default:
return UR_RESULT_ERROR_UNSUPPORTED_ENUMERATION;
return UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR;
}

// When a dimension isn't used pImageDesc has the size set to 1
Expand Down
2 changes: 1 addition & 1 deletion source/adapters/cuda/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urQueueCreateWithNativeHandle(
} else if (CuFlags == CU_STREAM_NON_BLOCKING) {
Flags = UR_QUEUE_FLAG_SYNC_WITH_DEFAULT_STREAM;
} else {
return UR_RESULT_ERROR_INVALID_OPERATION;
assert(!"Unknown cuda stream.");
}

std::vector<CUstream> ComputeCuStreams(1, CuStream);
Expand Down
4 changes: 1 addition & 3 deletions source/adapters/cuda/sampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,7 @@ UR_APIEXPORT ur_result_t UR_APICALL
urSamplerRelease(ur_sampler_handle_t hSampler) {
// double delete or someone is messing with the ref count.
// either way, cannot safely proceed.
if (hSampler->getReferenceCount() == 0) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert (hSampler->getReferenceCount() == 0 && "Reference count overflow detected in urSamplerRelease.");

// decrement ref count. If it is 0, delete the sampler.
if (hSampler->decrementReferenceCount() == 0) {
Expand Down
5 changes: 4 additions & 1 deletion source/adapters/hip/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,19 +179,22 @@ ur_result_t urGetLastResult(ur_platform_handle_t, const char **ppMessage) {
return ErrorMessageCode;
}

ur_result_t GetHipFormatPixelSize(hipArray_Format Format, int *Size) {
ur_result_t imageElementByteSize(hipArray_Format Format, unsigned int *Size) {
switch (Format) {
case HIP_AD_FORMAT_UNSIGNED_INT8:
case HIP_AD_FORMAT_SIGNED_INT8:
*Size = 1;
break;
case HIP_AD_FORMAT_UNSIGNED_INT16:
case HIP_AD_FORMAT_SIGNED_INT16:
case HIP_AD_FORMAT_HALF:
*Size = 2;
break;
case HIP_AD_FORMAT_UNSIGNED_INT32:
case HIP_AD_FORMAT_SIGNED_INT32:
case HIP_AD_FORMAT_FLOAT:
*Size = 4;
break;
default:
return UR_RESULT_ERROR_IMAGE_FORMAT_NOT_SUPPORTED;
}
Expand Down
8 changes: 4 additions & 4 deletions source/adapters/hip/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include <ur/ur.hpp>

/**
* Call an UR API and, if the result is not UR_RESULT_SUCCESS, automatically
* Call a UR API and, if the result is not UR_RESULT_SUCCESS, automatically
* return from the current function.
*/
#define UR_RETURN_ON_FAILURE(urCall) \
Expand Down Expand Up @@ -189,8 +189,8 @@ template <typename T> class ReleaseGuard {
// HIP error for which it is unclear if the function that reported it
// succeeded or not. Either way, the state of the program is compromised
// and likely unrecoverable.
detail::ur::hipPrint(
"Unrecoverable program state reached in piMemRelease");
assert(
!"Unrecoverable program state reached in piMemRelease");
}
}
}
Expand All @@ -208,4 +208,4 @@ template <typename T> class ReleaseGuard {
void dismiss() { Captive = nullptr; }
};

ur_result_t GetHipFormatPixelSize(hipArray_Format Format, int *Size);
ur_result_t imageElementByteSize(hipArray_Format Format, int *Size);
2 changes: 1 addition & 1 deletion source/adapters/hip/device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
UR_CHECK_ERROR(hipDeviceGetAttribute(
&EccEnabled, hipDeviceAttributeEccEnabled, hDevice->get()));

if ((EccEnabled != 0) | (EccEnabled != 1)) {
if ((EccEnabled != 0) || (EccEnabled != 1)) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
auto Result = static_cast<bool>(EccEnabled);
Expand Down
6 changes: 3 additions & 3 deletions source/adapters/hip/enqueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageRead(
UR_CHECK_ERROR(getArrayDesc(Array, Format, NumChannels));

int ElementByteSize = 0;
UR_RETURN_ON_FAILURE(GetHipFormatPixelSize(Format, &ElementByteSize));
UR_RETURN_ON_FAILURE(imageElementByteSize(Format, &ElementByteSize));

size_t ByteOffsetX = origin.x * ElementByteSize * NumChannels;
size_t BytesToCopy = ElementByteSize * NumChannels * region.depth;
Expand Down Expand Up @@ -1119,7 +1119,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageWrite(
UR_CHECK_ERROR(getArrayDesc(Array, Format, NumChannels));

int ElementByteSize = 0;
UR_RETURN_ON_FAILURE(GetHipFormatPixelSize(Format, &ElementByteSize));
UR_RETURN_ON_FAILURE(imageElementByteSize(Format, &ElementByteSize));

size_t ByteOffsetX = origin.x * ElementByteSize * NumChannels;
size_t BytesToCopy = ElementByteSize * NumChannels * region.depth;
Expand Down Expand Up @@ -1194,7 +1194,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueMemImageCopy(
UR_RESULT_ERROR_INVALID_IMAGE_FORMAT_DESCRIPTOR);

int ElementByteSize = 0;
UR_RETURN_ON_FAILURE(GetHipFormatPixelSize(SrcFormat, &ElementByteSize));
UR_RETURN_ON_FAILURE(imageElementByteSize(SrcFormat, &ElementByteSize));

size_t DstByteOffsetX = dstOrigin.x * ElementByteSize * SrcNumChannels;
size_t SrcByteOffsetX = srcOrigin.x * ElementByteSize * DstNumChannels;
Expand Down
12 changes: 3 additions & 9 deletions source/adapters/hip/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ ur_result_t ur_event_handle_t_::record() {

try {
EventId = Queue->getNextEventId();
if (EventId == 0) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert (EventId == 0 && "Unrecoverable program state reached in event identifier overflow.");
UR_CHECK_ERROR(hipEventRecord(EvEnd, Stream));
Result = UR_RESULT_SUCCESS;
} catch (ur_result_t Error) {
Expand Down Expand Up @@ -278,19 +276,15 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventSetCallback(ur_event_handle_t,
UR_APIEXPORT ur_result_t UR_APICALL urEventRetain(ur_event_handle_t hEvent) {
const auto RefCount = hEvent->incrementReferenceCount();

if (RefCount == 0) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(RefCount == 0 && "Reference count overflow detected in urEventRetain.");

return UR_RESULT_SUCCESS;
}

UR_APIEXPORT ur_result_t UR_APICALL urEventRelease(ur_event_handle_t hEvent) {
// double delete or someone is messing with the ref count.
// either way, cannot safely proceed.
if (hEvent->getReferenceCount() == 0) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(hEvent->getReferenceCount() == 0 && "Reference count overflow detected in urEventRelease.");

// decrement ref count. If it is 0, delete the event.
if (hEvent->decrementReferenceCount() == 0) {
Expand Down
8 changes: 4 additions & 4 deletions source/adapters/hip/memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemRelease(ur_mem_handle_t hMem) {
// error for which it is unclear if the function that reported it succeeded
// or not. Either way, the state of the program is compromised and likely
// unrecoverable.
return UR_RESULT_ERROR_INVALID_OPERATION;
assert(!"Unrecoverable program state reached in urMemRelease");
}

return UR_RESULT_SUCCESS;
Expand Down Expand Up @@ -246,9 +246,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemGetInfo(ur_mem_handle_t hMemory,
#else
HIP_ARRAY3D_DESCRIPTOR ArrayDescriptor;
UR_CHECK_ERROR(hipArray3DGetDescriptor(&ArrayDescriptor, Mem.Array));
auto PixelSize = 0;
int PixelSize = 0;
UR_RETURN_ON_FAILURE(
GetHipFormatPixelSize(ArrayDescriptor.Format, &PixelSize));
imageElementByteSize(ArrayDescriptor.Format, &PixelSize));
const auto PixelSizeBytes = PixelSize * ArrayDescriptor.NumChannels;
const auto ImageSizeBytes =
PixelSizeBytes *
Expand Down Expand Up @@ -397,7 +397,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urMemImageGetInfo(ur_mem_handle_t hMemory,
return ReturnValue(ArrayInfo.Depth);
case UR_IMAGE_INFO_ELEMENT_SIZE: {
int Size = 0;
UR_RETURN_ON_FAILURE(GetHipFormatPixelSize(ArrayInfo.Format, &Size));
UR_RETURN_ON_FAILURE(imageElementByteSize(ArrayInfo.Format, &Size));
return ReturnValue(Size);
}
case UR_IMAGE_INFO_ROW_PITCH:
Expand Down
2 changes: 1 addition & 1 deletion source/adapters/hip/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urQueueCreateWithNativeHandle(
} else if (HIPFlags == hipStreamNonBlocking) {
Flags = UR_QUEUE_FLAG_SYNC_WITH_DEFAULT_STREAM;
} else {
return UR_RESULT_ERROR_INVALID_OPERATION;
assert(!"Unknown hip stream.");
}

std::vector<hipStream_t> ComputeHIPStreams(1, HIPStream);
Expand Down
4 changes: 1 addition & 3 deletions source/adapters/hip/sampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@ ur_result_t urSamplerRetain(ur_sampler_handle_t hSampler) {
ur_result_t urSamplerRelease(ur_sampler_handle_t hSampler) {
// double delete or someone is messing with the ref count.
// either way, cannot safely proceed.
if (hSampler->getReferenceCount() == 0) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(hSampler->getReferenceCount() == 0 && "Reference count overflow detected in urSamplerRelease.");

// decrement ref count. If it is 0, delete the sampler.
if (hSampler->decrementReferenceCount() == 0) {
Expand Down
4 changes: 1 addition & 3 deletions source/adapters/level_zero/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -566,9 +566,7 @@ ur_context_handle_t_::decrementUnreleasedEventsInPool(ur_event_handle_t Event) {
getZeEventPoolCache(Event->isHostVisible(), Event->isProfilingEnabled());

// Put the empty pool to the cache of the pools.
if (NumEventsUnreleasedInEventPool[Event->ZeEventPool] == 0) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(NumEventsUnreleasedInEventPool[Event->ZeEventPool] == 0 && "Invalid event release: event pool doesn't have unreleased events");
if (--NumEventsUnreleasedInEventPool[Event->ZeEventPool] == 0) {
if (ZePoolCache->front() != Event->ZeEventPool) {
ZePoolCache->push_back(Event->ZeEventPool);
Expand Down
16 changes: 4 additions & 12 deletions source/adapters/level_zero/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -585,9 +585,7 @@ ur_result_t ur_event_handle_t_::getOrCreateHostVisibleEvent(
this->Mutex);

if (!HostVisibleEvent) {
if (UrQueue->ZeEventsScope != OnDemandHostVisibleProxy) {
return UR_RESULT_ERROR_INVALID_EVENT;
}
assert(UrQueue->ZeEventsScope != OnDemandHostVisibleProxy && "getOrCreateHostVisibleEvent: missing host-visible event");

// Submit the command(s) signalling the proxy event to the queue.
// We have to first submit a wait for the device-only event for which this
Expand Down Expand Up @@ -633,9 +631,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventWait(
//
ur_event_handle_t_ *Event =
ur_cast<ur_event_handle_t_ *>(EventWaitList[I]);
if (!Event->hasExternalRefs()) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(!Event->hasExternalRefs() && "urEventsWait must not be called for an internal event");

ze_event_handle_t ZeHostVisibleEvent;
if (auto Res = Event->getOrCreateHostVisibleEvent(ZeHostVisibleEvent))
Expand All @@ -660,15 +656,11 @@ UR_APIEXPORT ur_result_t UR_APICALL urEventWait(
ur_cast<ur_event_handle_t_ *>(EventWaitList[I]);
{
std::shared_lock<ur_shared_mutex> EventLock(Event->Mutex);
if (!Event->hasExternalRefs()) {
return UR_RESULT_ERROR_INVALID_OPERATION;
}
assert(!Event->hasExternalRefs() && "urEventWait must not be called for an internal event");

if (!Event->Completed) {
auto HostVisibleEvent = Event->HostVisibleEvent;
if (!HostVisibleEvent) {
return UR_RESULT_ERROR_INVALID_EVENT;
}
assert(!HostVisibleEvent && "The host-visible proxy event missing");

ze_event_handle_t ZeEvent = HostVisibleEvent->ZeEvent;
urPrint("ZeEvent = %#llx\n", ur_cast<std::uintptr_t>(ZeEvent));
Expand Down
Loading

0 comments on commit ae9ae1e

Please sign in to comment.