Skip to content

Uts for commandFillBuffer, commandFillImage#19

Open
kamil-goras-mobica wants to merge 4 commits intomainfrom
ut_commandfillbuffer
Open

Uts for commandFillBuffer, commandFillImage#19
kamil-goras-mobica wants to merge 4 commits intomainfrom
ut_commandfillbuffer

Conversation

@kamil-goras-mobica
Copy link
Collaborator

No description provided.

@shajder
Copy link
Collaborator

shajder commented Oct 11, 2023

@kamil-goras-mobica recap of our discussion over this PR, conclusion is to split this PR into 6 smaller:
-commandFill...
-commandCopy...
-updateMutableDispatch
-commandNDRange
-finalizeCommandBuffer
-enqueueCommandBuffer

@kamil-goras-mobica kamil-goras-mobica changed the title Ut commandfillbuffer Uts for commandFillBuffer, commandFillImage Oct 12, 2023
#endif
}

cl_int clCommandFillBufferKHR_testcommandFillBuffer(cl_command_buffer_khr command_buffer, cl_command_queue command_queue, cl_mem buffer,
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's make that one static please

TEST_ASSERT_EQUAL(*sync_point_wait_list, 0);
TEST_ASSERT_EQUAL_PTR(mutable_handle, nullptr);
TEST_ASSERT_NOT_EQUAL(sync_point, nullptr);
return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could return CL_SUCCESS explicitly

cl_int ret = CL_INVALID_CONTEXT;
float pattern = 0;
size_t offset = 0;
size_t size = sizeof(pattern);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be size of the buffer to fill with the pattern, not the size of the pattern.

const void* pattern, size_t pattern_size, size_t offset, size_t size, cl_uint num_sync_points_in_wait_list,
const cl_sync_point_khr* sync_point_wait_list, cl_sync_point_khr* sync_point, cl_mutable_command_khr* mutable_handle, int num_calls)
{
(void)size;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Preference to set and verify real value different then size of pattern.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@shajder Added corrections to all comments

Copy link
Collaborator

@shajder shajder left a comment

Choose a reason for hiding this comment

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

LGTM

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