Skip to content

Commit 9b99015

Browse files
GeorgeWebGeorgeWeb
committed
Address review feedback
1 parent fa865c4 commit 9b99015

File tree

4 files changed

+24
-30
lines changed

4 files changed

+24
-30
lines changed

‎source/adapters/hip/device.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -880,27 +880,26 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
880880
return ReturnValue(ur_bool_t{false});
881881
}
882882
case UR_DEVICE_INFO_BINDLESS_SAMPLED_IMAGE_FETCH_1D_USM_EXP: {
883-
// HIP does support fetching 1D USM sampled image data.
883+
// HIP supports fetching 1D USM sampled image data.
884884
// TODO: DPC++ doesn't implement the required builtins for SYCL.
885885
return ReturnValue(ur_bool_t{false});
886886
}
887887
case UR_DEVICE_INFO_BINDLESS_SAMPLED_IMAGE_FETCH_1D_EXP: {
888888
// HIP does not support fetching 1D non-USM sampled image data.
889-
// TODO: DPC++ doesn't implement the required builtins for SYCL.
890889
return ReturnValue(ur_bool_t{false});
891890
}
892891
case UR_DEVICE_INFO_BINDLESS_SAMPLED_IMAGE_FETCH_2D_USM_EXP: {
893-
// HIP does support fetching 2D USM sampled image data.
892+
// HIP supports fetching 2D USM sampled image data.
894893
// TODO: DPC++ doesn't implement the required builtins for SYCL.
895894
return ReturnValue(ur_bool_t{false});
896895
}
897896
case UR_DEVICE_INFO_BINDLESS_SAMPLED_IMAGE_FETCH_2D_EXP: {
898-
// HIP does support fetching 2D non-USM sampled image data.
897+
// HIP supports fetching 2D non-USM sampled image data.
899898
// TODO: DPC++ doesn't implement the required builtins for SYCL.
900899
return ReturnValue(ur_bool_t{false});
901900
}
902901
case UR_DEVICE_INFO_BINDLESS_SAMPLED_IMAGE_FETCH_3D_EXP: {
903-
// HIP does support fetching 3D non-USM sampled image data.
902+
// HIP supports fetching 3D non-USM sampled image data.
904903
// TODO: DPC++ doesn't implement the required builtins for SYCL.
905904
return ReturnValue(ur_bool_t{false});
906905
}
@@ -916,12 +915,12 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
916915
return ReturnValue(ur_bool_t{false});
917916
}
918917
case UR_DEVICE_INFO_BINDLESS_SAMPLE_1D_USM_EXP: {
919-
// HIP does support sampling 1D USM sampled image data.
918+
// HIP supports sampling 1D USM sampled image data.
920919
return ReturnValue(
921920
static_cast<ur_bool_t>(hDevice->supportsHardwareImages()));
922921
}
923922
case UR_DEVICE_INFO_BINDLESS_SAMPLE_2D_USM_EXP: {
924-
// HIP does support sampling 2D USM sampled image data.
923+
// HIP supports sampling 2D USM sampled image data.
925924
return ReturnValue(
926925
static_cast<ur_bool_t>(hDevice->supportsHardwareImages()));
927926
}

‎source/adapters/hip/device.hpp

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313

1414
#include <ur/ur.hpp>
1515

16-
#include <map>
17-
1816
/// UR device mapping to a hipDevice_t.
1917
/// Includes an observer pointer to the platform,
2018
/// and implements the reference counting semantics since
@@ -36,7 +34,7 @@ struct ur_device_handle_t_ {
3634
int DeviceMaxLocalMem{0};
3735
int ManagedMemSupport{0};
3836
int ConcurrentManagedAccess{0};
39-
int HardwareImageSupport{0};
37+
bool HardwareImageSupport{false};
4038

4139
public:
4240
ur_device_handle_t_(native_type HipDevice, hipEvent_t EvBase,
@@ -61,9 +59,11 @@ struct ur_device_handle_t_ {
6159
&ConcurrentManagedAccess, hipDeviceAttributeConcurrentManagedAccess,
6260
HIPDevice));
6361
// Check if texture functions are supported in the HIP host runtime.
64-
UR_CHECK_ERROR(hipDeviceGetAttribute(
65-
&HardwareImageSupport, hipDeviceAttributeImageSupport, HIPDevice));
66-
detail::ur::assertion(HardwareImageSupport >= 0);
62+
int Ret{};
63+
UR_CHECK_ERROR(
64+
hipDeviceGetAttribute(&Ret, hipDeviceAttributeImageSupport, HIPDevice));
65+
detail::ur::assertion(Ret == 0 || Ret == 1);
66+
HardwareImageSupport = Ret == 1;
6767
}
6868

6969
~ur_device_handle_t_() noexcept(false) {}
@@ -96,12 +96,7 @@ struct ur_device_handle_t_ {
9696
return ConcurrentManagedAccess;
9797
};
9898

99-
bool supportsHardwareImages() const noexcept {
100-
return HardwareImageSupport ? true : false;
101-
}
102-
103-
// Used for bookkeeping for mipmapped array leaks in mapping external memory.
104-
std::map<hipArray_t, hipMipmappedArray_t> ChildHipArrayFromMipmapMap;
99+
bool supportsHardwareImages() const noexcept { return HardwareImageSupport; }
105100
};
106101

107102
int getAttribute(ur_device_handle_t Device, hipDeviceAttribute_t Attribute);

‎source/adapters/hip/image.cpp

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ hipToUrImageChannelFormat(hipArray_Format hip_format,
128128
*return_image_channel_type = TO; \
129129
return UR_RESULT_SUCCESS; \
130130
}
131+
131132
HIP_TO_UR_IMAGE_CHANNEL_TYPE(HIP_AD_FORMAT_UNSIGNED_INT8,
132133
UR_IMAGE_CHANNEL_TYPE_UNSIGNED_INT8);
133134
HIP_TO_UR_IMAGE_CHANNEL_TYPE(HIP_AD_FORMAT_UNSIGNED_INT16,
@@ -144,6 +145,8 @@ hipToUrImageChannelFormat(hipArray_Format hip_format,
144145
UR_IMAGE_CHANNEL_TYPE_HALF_FLOAT);
145146
HIP_TO_UR_IMAGE_CHANNEL_TYPE(HIP_AD_FORMAT_FLOAT,
146147
UR_IMAGE_CHANNEL_TYPE_FLOAT);
148+
149+
#undef HIP_TO_UR_IMAGE_CHANNEL_TYPE
147150
default:
148151
return UR_RESULT_ERROR_UNSUPPORTED_IMAGE_FORMAT;
149152
}
@@ -156,7 +159,6 @@ ur_result_t urTextureCreate(ur_sampler_handle_t hSampler,
156159
ur_exp_image_native_handle_t *phRetImage) {
157160

158161
try {
159-
/// pi_sampler_properties
160162
/// Layout of UR samplers for HIP
161163
///
162164
/// Sampler property layout:
@@ -425,12 +427,6 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageFreeExp(
425427
try {
426428
hipArray_t ImageArray = reinterpret_cast<hipArray_t>(hImageMem);
427429
UR_CHECK_ERROR(hipArrayDestroy(ImageArray));
428-
if (auto It = hDevice->ChildHipArrayFromMipmapMap.find(ImageArray);
429-
It != hDevice->ChildHipArrayFromMipmapMap.end()) {
430-
UR_CHECK_ERROR(hipMipmappedArrayDestroy(
431-
static_cast<hipMipmappedArray_t>(It->second)));
432-
hDevice->ChildHipArrayFromMipmapMap.erase(It);
433-
}
434430
} catch (ur_result_t Err) {
435431
return Err;
436432
} catch (...) {
@@ -625,7 +621,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageCopyExp(
625621

626622
if (memType == hipMemoryTypeArray) {
627623
// HIP doesn not provide async copies between host and image arrays
628-
// memory in versions early than 6.2.
624+
// memory in versions earlier than 6.2.
629625
#if HIP_VERSION >= 60200000
630626
UR_CHECK_ERROR(
631627
hipMemcpyHtoAAsync(static_cast<hipArray_t>(pDst),
@@ -733,7 +729,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImageCopyExp(
733729

734730
if (memType == hipMemoryTypeArray) {
735731
// HIP doesn not provide async copies between image arrays and host
736-
// memory in versions early than 6.2.
732+
// memory in versions earlier than 6.2.
737733
#if HIP_VERSION >= 60200000
738734
UR_CHECK_ERROR(hipMemcpyAtoHAsync(
739735
DstWithOffset, static_cast<hipArray_t>(const_cast<void *>(pSrc)),
@@ -1095,8 +1091,9 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImportExternalMemoryExp(
10951091
extMemDesc.type = hipExternalMemoryHandleTypeOpaqueWin32;
10961092
break;
10971093
case UR_EXP_EXTERNAL_MEM_TYPE_WIN32_NT_DX12_RESOURCE:
1098-
// Memory descriptor flag values such as hipExternalMemoryDedicatedare
1099-
// not available before HIP 5.6, so we safely fallback to unsupported.
1094+
// Memory descriptor flag values such as hipExternalMemoryDedicated
1095+
// are not available before HIP 5.6, so we safely fallback to marking
1096+
// this as an unsupported.
11001097
#if HIP_VERSION >= 50600000
11011098
extMemDesc.type = hipExternalMemoryHandleTypeD3D12Resource;
11021099
extMemDesc.flags = hipExternalMemoryDedicated;
@@ -1230,6 +1227,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urBindlessImagesImportExternalSemaphoreExp(
12301227
extSemDesc.type = hipExternalSemaphoreHandleTypeD3D12Fence;
12311228
break;
12321229
case UR_EXP_EXTERNAL_SEMAPHORE_TYPE_OPAQUE_FD:
1230+
[[fallthrough]];
12331231
default:
12341232
return UR_RESULT_ERROR_INVALID_VALUE;
12351233
}

‎source/adapters/hip/sampler.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ struct ur_sampler_handle_t_ {
5555
}
5656

5757
ur_sampler_addressing_mode_t getAddressingModeDim(size_t i) const noexcept {
58+
// valid dimensions are 0,1,2
59+
assert(i < 3);
5860
return static_cast<ur_sampler_addressing_mode_t>((Props >> (2 + (i * 3))) &
5961
0b111);
6062
}

0 commit comments

Comments
 (0)