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

[L0] coverity fixes #1179

Merged
merged 1 commit into from
Dec 13, 2023
Merged
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
11 changes: 4 additions & 7 deletions source/adapters/level_zero/adapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,17 +156,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urAdapterRetain(ur_adapter_handle_t) {
}

UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetLastError(
[[maybe_unused]] ur_adapter_handle_t
AdapterHandle, ///< [in] handle of the platform instance
ur_adapter_handle_t, ///< [in] handle of the platform instance
const char **Message, ///< [out] pointer to a C string where the adapter
///< specific error message will be stored.
[[maybe_unused]] int32_t
*Error ///< [out] pointer to an integer where the adapter specific
///< error code will be stored.
int32_t *Error ///< [out] pointer to an integer where the adapter specific
///< error code will be stored.
) {
AdapterHandle = &Adapter;
*Message = ErrorMessage;
Error = &ErrorAdapterNativeCode;
*Error = ErrorAdapterNativeCode;

return ErrorMessageCode;
}
Expand Down
19 changes: 10 additions & 9 deletions source/adapters/level_zero/device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ur_level_zero.hpp"
#include <algorithm>
#include <climits>
#include <optional>

UR_APIEXPORT ur_result_t UR_APICALL urDeviceGet(
ur_platform_handle_t Platform, ///< [in] handle of the platform instance
Expand Down Expand Up @@ -353,8 +354,10 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(
UR_DEVICE_AFFINITY_DOMAIN_FLAG_NEXT_PARTITIONABLE));
case UR_DEVICE_INFO_PARTITION_TYPE: {
// For root-device there is no partitioning to report.
if (pSize && !Device->isSubDevice()) {
*pSize = 0;
if (Device->SubDeviceCreationProperty == std::nullopt ||
!Device->isSubDevice()) {
if (pSize)
*pSize = 0;
return UR_RESULT_SUCCESS;
}

Expand All @@ -365,7 +368,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(
return ReturnValue(cslice);
}

return ReturnValue(Device->SubDeviceCreationProperty);
return ReturnValue(*Device->SubDeviceCreationProperty);
}
// Everything under here is not supported yet
case UR_EXT_DEVICE_INFO_OPENCL_C_VERSION:
Expand Down Expand Up @@ -1218,16 +1221,14 @@ UR_APIEXPORT ur_result_t UR_APICALL urDevicePartition(
UR_ASSERT(NumDevices == EffectiveNumDevices, UR_RESULT_ERROR_INVALID_VALUE);

for (uint32_t I = 0; I < NumDevices; I++) {
Device->SubDevices[I]->SubDeviceCreationProperty =
Properties->pProperties[0];
if (Properties->pProperties[0].type ==
UR_DEVICE_PARTITION_BY_AFFINITY_DOMAIN) {
auto prop = Properties->pProperties[0];
if (prop.type == UR_DEVICE_PARTITION_BY_AFFINITY_DOMAIN) {
// In case the value is NEXT_PARTITIONABLE, we need to change it to the
// chosen domain. This will always be NUMA since that's the only domain
// supported by level zero.
Device->SubDevices[I]->SubDeviceCreationProperty.value.affinity_domain =
UR_DEVICE_AFFINITY_DOMAIN_FLAG_NUMA;
prop.value.affinity_domain = UR_DEVICE_AFFINITY_DOMAIN_FLAG_NUMA;
}
Device->SubDevices[I]->SubDeviceCreationProperty = prop;

OutDevices[I] = Device->SubDevices[I];
// reusing the same pi_device needs to increment the reference count
Expand Down
3 changes: 2 additions & 1 deletion source/adapters/level_zero/device.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <cassert>
#include <list>
#include <map>
#include <optional>
#include <stdarg.h>
#include <string>
#include <unordered_map>
Expand Down Expand Up @@ -116,7 +117,7 @@ struct ur_device_handle_t_ : _ur_object {

// If this device is a subdevice, this variable contains the properties that
// were used during its creation.
ur_device_partition_property_t SubDeviceCreationProperty;
std::optional<ur_device_partition_property_t> SubDeviceCreationProperty;

// PI platform to which this device belongs.
// This field is only set at _ur_device_handle_t creation time, and cannot
Expand Down
6 changes: 4 additions & 2 deletions source/adapters/level_zero/kernel.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
struct ur_kernel_handle_t_ : _ur_object {
ur_kernel_handle_t_(ze_kernel_handle_t Kernel, bool OwnZeHandle,
ur_program_handle_t Program)
: Program{Program}, ZeKernel{Kernel}, SubmissionsCount{0}, MemAllocs{} {
: Context{nullptr}, Program{Program}, ZeKernel{Kernel},
SubmissionsCount{0}, MemAllocs{} {
OwnNativeHandle = OwnZeHandle;
}

ur_kernel_handle_t_(ze_kernel_handle_t Kernel, bool OwnZeHandle,
ur_context_handle_t Context)
: Context{Context}, ZeKernel{Kernel}, SubmissionsCount{0}, MemAllocs{} {
: Context{Context}, Program{nullptr}, ZeKernel{Kernel},
SubmissionsCount{0}, MemAllocs{} {
OwnNativeHandle = OwnZeHandle;
}

Expand Down
10 changes: 5 additions & 5 deletions source/adapters/level_zero/memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2078,9 +2078,9 @@ ur_result_t _ur_buffer::getZeHandle(char *&ZeHandle, access_mode_t AccessMode,
auto &Allocation = Allocations[Device];

// Sub-buffers don't maintain own allocations but rely on parent buffer.
if (isSubBuffer()) {
UR_CALL(SubBuffer.Parent->getZeHandle(ZeHandle, AccessMode, Device));
ZeHandle += SubBuffer.Origin;
if (SubBuffer) {
UR_CALL(SubBuffer->Parent->getZeHandle(ZeHandle, AccessMode, Device));
ZeHandle += SubBuffer->Origin;
// Still store the allocation info in the PI sub-buffer for
// getZeHandlePtr to work. At least zeKernelSetArgumentValue needs to
// be given a pointer to the allocation handle rather than its value.
Expand Down Expand Up @@ -2312,7 +2312,7 @@ ur_result_t _ur_buffer::free() {
// Buffer constructor
_ur_buffer::_ur_buffer(ur_context_handle_t Context, size_t Size, char *HostPtr,
bool ImportedHostPtr = false)
: ur_mem_handle_t_(Context), Size(Size), SubBuffer{nullptr, 0} {
: ur_mem_handle_t_(Context), Size(Size) {

// We treat integrated devices (physical memory shared with the CPU)
// differently from discrete devices (those with distinct memories).
Expand Down Expand Up @@ -2347,7 +2347,7 @@ _ur_buffer::_ur_buffer(ur_context_handle_t Context, ur_device_handle_t Device,
_ur_buffer::_ur_buffer(ur_context_handle_t Context, size_t Size,
ur_device_handle_t Device, char *ZeMemHandle,
bool OwnZeMemHandle)
: ur_mem_handle_t_(Context, Device), Size(Size), SubBuffer{nullptr, 0} {
: ur_mem_handle_t_(Context, Device), Size(Size) {

// Device == nullptr means host allocation
Allocations[Device].ZeHandle = ZeMemHandle;
Expand Down
16 changes: 9 additions & 7 deletions source/adapters/level_zero/memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <cassert>
#include <list>
#include <map>
#include <optional>
#include <stdarg.h>
#include <string>
#include <unordered_map>
Expand Down Expand Up @@ -84,7 +85,8 @@ struct ur_mem_handle_t_ : _ur_object {
virtual ~ur_mem_handle_t_() = default;

protected:
ur_mem_handle_t_(ur_context_handle_t Context) : UrContext{Context} {}
ur_mem_handle_t_(ur_context_handle_t Context)
: UrContext{Context}, UrDevice{nullptr} {}

ur_mem_handle_t_(ur_context_handle_t Context, ur_device_handle_t Device)
: UrContext{Context}, UrDevice(Device) {}
Expand All @@ -101,7 +103,7 @@ struct _ur_buffer final : ur_mem_handle_t_ {
// Sub-buffer constructor
_ur_buffer(_ur_buffer *Parent, size_t Origin, size_t Size)
: ur_mem_handle_t_(Parent->UrContext),
Size(Size), SubBuffer{Parent, Origin} {}
Size(Size), SubBuffer{{Parent, Origin}} {}

// Interop-buffer constructor
_ur_buffer(ur_context_handle_t Context, size_t Size,
Expand All @@ -121,8 +123,7 @@ struct _ur_buffer final : ur_mem_handle_t_ {
ur_device_handle_t Device = nullptr) override;

bool isImage() const override { return false; }

bool isSubBuffer() const { return SubBuffer.Parent != nullptr; }
bool isSubBuffer() const { return SubBuffer != std::nullopt; }

// Frees all allocations made for the buffer.
ur_result_t free();
Expand Down Expand Up @@ -174,10 +175,11 @@ struct _ur_buffer final : ur_mem_handle_t_ {
size_t Size;
size_t getAlignment() const;

struct {
struct SubBuffer_t {
_ur_buffer *Parent;
size_t Origin; // only valid if Parent != nullptr
} SubBuffer;
size_t Origin;
};
std::optional<SubBuffer_t> SubBuffer;
};

struct _ur_image final : ur_mem_handle_t_ {
Expand Down
4 changes: 3 additions & 1 deletion source/adapters/level_zero/platform.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@
#pragma once

#include "common.hpp"
#include "ze_api.h"

struct ur_device_handle_t_;

struct ur_platform_handle_t_ : public _ur_platform {
ur_platform_handle_t_(ze_driver_handle_t Driver) : ZeDriver{Driver} {}
ur_platform_handle_t_(ze_driver_handle_t Driver)
: ZeDriver{Driver}, ZeApiVersion{ZE_API_VERSION_CURRENT} {}
// Performs initialization of a newly constructed PI platform.
ur_result_t initialize();

Expand Down
6 changes: 3 additions & 3 deletions source/adapters/level_zero/queue.cpp
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urQueueGetInfo(
if (ImmCmdList == Queue->CommandListMap.end())
continue;

auto EventList = ImmCmdList->second.EventList;
const auto &EventList = ImmCmdList->second.EventList;
for (auto It = EventList.crbegin(); It != EventList.crend(); It++) {
ze_result_t ZeResult =
ZE_CALL_NOCHECK(zeEventQueryStatus, ((*It)->ZeEvent));
Expand Down Expand Up @@ -391,11 +391,11 @@ UR_APIEXPORT ur_result_t UR_APICALL urQueueCreate(
// At this point only the thread creating the queue will have associated
// command-lists. Other threads have not accessed the queue yet. So we can
// only warmup the initial thread's command-lists.
auto QueueGroup = Q->ComputeQueueGroupsByTID.get();
const auto &QueueGroup = Q->ComputeQueueGroupsByTID.get();
UR_CALL(warmupQueueGroup(false, QueueGroup.UpperIndex -
QueueGroup.LowerIndex + 1));
if (Q->useCopyEngine()) {
auto QueueGroup = Q->CopyQueueGroupsByTID.get();
const auto &QueueGroup = Q->CopyQueueGroupsByTID.get();
UR_CALL(warmupQueueGroup(true, QueueGroup.UpperIndex -
QueueGroup.LowerIndex + 1));
}
Expand Down
3 changes: 2 additions & 1 deletion source/adapters/level_zero/queue.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,8 @@ struct ur_queue_handle_t_ : _ur_object {
// checked. Otherwise, the OpenCommandList containing compute commands is
// checked.
bool hasOpenCommandList(bool IsCopy) const {
auto CommandBatch = (IsCopy) ? CopyCommandBatch : ComputeCommandBatch;
const auto &CommandBatch =
(IsCopy) ? CopyCommandBatch : ComputeCommandBatch;
return CommandBatch.OpenCommandList != CommandListMap.end();
}

Expand Down