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

Add missing image types to image tests #155

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

oskarweb
Copy link

@oskarweb oskarweb commented Feb 18, 2025

  • Add 1DARRAY, 2DARRAY to layout tests
  • Add 1D, 3D, 1DARRAY, 2DARRAY to swizzle, format, view tests
  • Add kernels for specific types

Signed-off-by: Oskar Hubert Weber [email protected]

@darmroz darmroz requested review from KapiX and marekkozl February 20, 2025 08:58
Comment on lines +26 to +27
LOG_INFO << "device does not support images, cannot run test";
GTEST_SKIP();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LOG_INFO << "device does not support images, cannot run test";
GTEST_SKIP();
GTEST_SKIP() << "Device does not support images";

if (properties.maxImageDims1D > 0) {
supported_types.emplace_back(ZE_IMAGE_TYPE_1D);
} else {
LOG_INFO << "ZE_IMAGE_TYPE_1D unsupported";
Copy link
Contributor

Choose a reason for hiding this comment

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

if you want convert L0 value to string use utils_string.cpp for that. example std::string to_string(const ze_image_type_t type)

Comment on lines +95 to +99
auto result = zeDeviceGetImageProperties(device, &properties);

if (result != ZE_RESULT_SUCCESS) {
return {};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
auto result = zeDeviceGetImageProperties(device, &properties);
if (result != ZE_RESULT_SUCCESS) {
return {};
}
EXPECT_EQ(ZE_RESULT_SUCCESS, zeDeviceGetImageProperties(device, &properties));

Copy link
Contributor

Choose a reason for hiding this comment

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

better to use harness function get_image_properties

Comment on lines +92 to +94
ze_device_image_properties_t properties;
memset(&properties, 0, sizeof(properties));
properties.stype = ZE_STRUCTURE_TYPE_IMAGE_PROPERTIES;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ze_device_image_properties_t properties;
memset(&properties, 0, sizeof(properties));
properties.stype = ZE_STRUCTURE_TYPE_IMAGE_PROPERTIES;
ze_device_image_properties_t properties = {};
properties.stype = ZE_STRUCTURE_TYPE_IMAGE_PROPERTIES;

Comment on lines +86 to +89
std::vector<ze_image_type_t> get_supported_image_types(
ze_device_handle_t device,
bool exclude_arrays,
bool exclude_buffer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

harness functions must be simple, if addition logic is required adapt helper functions in test itself

test.outbuff = lzt::allocate_host_memory(test.image_size * sizeof(float));
float *in_ptr = static_cast<float *>(test.inbuff);
for (int i = 0; i < test.image_size; i++) {
in_ptr[i] = 3.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

please move 3.5 value to a constexpr variable with the name explaining the value


ze_group_count_t group_dems = {image_width / group_size_x,
image_height / group_size_y, 1};
result = zeKernelSetArgumentValue(kernel, 0, sizeof(img_in), &img_in);
Copy link
Contributor

Choose a reason for hiding this comment

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

use existing functions from module harness

Comment on lines +245 to 250
auto result = zeImageCreate(lzt::get_default_context(),
lzt::zeDevice::get_instance()->get_device(),
&image_desc, &image);
EXPECT_EQ(ZE_RESULT_SUCCESS,
zeImageCreate(lzt::get_default_context(),
lzt::zeDevice::get_instance()->get_device(),
Copy link
Contributor

Choose a reason for hiding this comment

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

i dont think this is expected behavior, either way please use harness image functions

Comment on lines +97 to +98
LOG_INFO << "RUN";
LOG_INFO << img_type;
Copy link
Contributor

Choose a reason for hiding this comment

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

either provide a meaningful message or don't log at all

Comment on lines +154 to +162
virtual void check_extensions() override {
if (!lzt::check_if_extension_supported(lzt::get_default_driver(),
ZE_IMAGE_VIEW_EXT_NAME)) {
GTEST_SKIP() << "Missing ZE_extension_image_view support\n";
}
if (!lzt::check_if_extension_supported(lzt::get_default_driver(),
ZE_IMAGE_VIEW_PLANAR_EXT_NAME)) {
GTEST_SKIP() << "Missing ZE_extension_image_view_planar support\n";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this method will provide a result SKIP, but won't cause test to end continuing to create unsupported image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants